Skip to content

ROX-9350 Use fine-grained host paths for compliance mounts #2479

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

Merged
robbycochran merged 6 commits into from
Nov 16, 2022
Merged

ROX-9350 Use fine-grained host paths for compliance mounts #2479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
813ba7b
X-Smart-Branch-Parent: origin/master
robbycochran Nov 16, 2022
a21297f
ROX-9350 - Use targeted paths for compliance mounts
robbycochran Jul 21, 2022
d611bf7
use subPath for shared mounts
robbycochran Jul 22, 2022
f868c64
use relative paths
robbycochran Jul 22, 2022
cfc82b5
Add /host/lib
robbycochran Nov 15, 2022
af27644
Add changelog
robbycochran Nov 16, 2022
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
Expand Up @@ -53,6 +53,7 @@ Please avoid adding duplicate information across this changelog and JIRA/doc inp
- ROX-13500: Previously, deployment YAML check on V1 CronJob workload would cause Central to panic. This is now fixed.
- `cves.ids` field of `storage.VulnerabilityRequest` object, which is in the response of `VulnerabilityRequestService` (`/v1/cve/requests/`) endpoints, has been renamed to `cves.cves`.
- ROX-13347: Vulnerability reporting scopes specifying cluster and/or namespace names now perform exact matches on those entities, as opposed to the erroneous prefix match.
- ROX-9350: The compliance container no longer mounts the entire host root to prevent a recursive mount of other pods' persistent volumes.

## [3.72.0]

Expand Down
87 changes: 73 additions & 14 deletions image/templates/helm/stackrox-secured-cluster/templates/collector.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ spec:
readOnlyRootFilesystem: true
volumeMounts:
- mountPath: /host/var/run/docker.sock
name: var-run-docker-sock
name: var-run
subPath: var/run/docker.sock
readOnly: true
- mountPath: /host/proc
name: proc-ro
Expand All @@ -76,7 +77,8 @@ spec:
name: etc-ro
readOnly: true
- mountPath: /host/usr/lib
name: usr-lib-ro
name: usr-ro
subPath: usr/lib
readOnly: true
- mountPath: /host/sys
name: sys-ro
Expand Down Expand Up @@ -116,16 +118,49 @@ spec:
name: etc-ssl
- mountPath: /etc/pki/ca-trust/
name: etc-pki-volume
- mountPath: /host
name: host-root-ro
- mountPath: /host/etc
name: etc-ro
readOnly: true
- mountPath: /host/proc
name: proc-ro
readOnly: true
- mountPath: /host/opt
name: opt-ro
readOnly: true
- mountPath: /host/run
name: run-ro
readOnly: true
- mountPath: /host/srv
name: srv-ro
readOnly: true
- mountPath: /host/sys
name: sys-ro
readOnly: true
- mountPath: /host/usr
name: usr-ro
readOnly: true
Comment on lines +139 to +141
Copy link
Copy Markdown
Contributor

@vikin91 vikin91 Oct 27, 2022

Choose a reason for hiding this comment

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

I am wondering why do we mount now /usr when we used to mount only /usr/lib. Is this intended?

Copy link
Copy Markdown
Contributor Author

@robbycochran robbycochran Nov 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It is intended. The collector and compliance containers now share volumes. The mount directories for the collector container are not changed, just the volumes used to provide those mounts. Previously, collector directly mounted /usr/lib via the host /usr/lib volume, now we mount via subPath on the host /usr volume mount that is shared with the compliance container.

Also note that previously, the compliance container mounted the entire host root.

- mountPath: /host/lib
name: lib-ro
readOnly: true
- mountPath: /host/var/lib/kubelet/kubeconfig
name: var-lib-kubelet-kubeconfig
readOnly: true
- mountPath: /host/var/lib/docker
name: var-lib-docker
readOnly: true
- mountPath: /host/var/lib/containers
name: var-lib-containers
readOnly: true
- mountPath: /host/var/log
name: var-log
readOnly: true
- mountPath: /host/var/run
name: var-run
readOnly: true
- mountPath: /run/secrets/stackrox.io/certs/
name: certs
readOnly: true
volumes:
- hostPath:
path: /var/run/docker.sock
name: var-run-docker-sock
- hostPath:
path: /proc
name: proc-ro
Expand All @@ -136,14 +171,41 @@ spec:
path: /etc
name: etc-ro
- hostPath:
path: /usr/lib
name: usr-lib-ro
- hostPath:
path: /sys/
path: /sys
name: sys-ro
- hostPath:
path: /dev
name: dev-ro
- hostPath:
path: /opt
name: opt-ro
- hostPath:
path: /run
name: run-ro
- hostPath:
path: /srv
name: srv-ro
- hostPath:
path: /usr
name: usr-ro
- hostPath:
path: /lib
name: lib-ro
- hostPath:
path: /var/lib/kubelet/kubeconfig
name: var-lib-kubelet-kubeconfig
- hostPath:
path: /var/lib/docker
name: var-lib-docker
- hostPath:
path: /var/lib/containers
name: var-lib-containers
- hostPath:
path: /var/log
name: var-log
- hostPath:
path: /var/run
name: var-run
- name: certs
secret:
secretName: collector-tls
Expand All @@ -154,9 +216,6 @@ spec:
path: key.pem
- key: ca.pem
path: ca.pem
- hostPath:
Copy link
Copy Markdown
Contributor

@RTann RTann Nov 15, 2022

Choose a reason for hiding this comment

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

note: if we do decide to support language-level vulnerabilities in node-scanning, then will will probably need to mount / again, as these packages can, in theory, be anywhere

Copy link
Copy Markdown
Contributor Author

@robbycochran robbycochran Nov 15, 2022

Choose a reason for hiding this comment

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

Yes that's a concern. The key issue is the fact that the compliance container does long-running mounts. Have we thought about node-scanning in an "on-demand" model, where the node scanning occurs with a short-term mount of the root directory?

Copy link
Copy Markdown
Contributor

@RTann RTann Nov 17, 2022

Choose a reason for hiding this comment

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

We could consider that. I'm not familiar with this concept at all, but we can discuss more when/if we need to add language-level vulns

path: /
name: host-root-ro
- name: etc-ssl
emptyDir: {}
- name: etc-pki-volume
Expand Down