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.

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

fix: make etc binds read-only #10155

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Conversation

frezbo
Copy link
Member

@frezbo frezbo commented Jan 16, 2025

Fix the /etc bind mounts read-only by calling a remount as ro.

The proper fix is to use new fsopen and fsmount API's.

@frezbo
Copy link
Member Author

frezbo commented Jan 16, 2025

Before:

❯ talosctl -n 10.5.0.3 read /proc/mounts | grep "/etc"
tmpfs /etc/cri/conf.d/hosts tmpfs rw,seclabel,relatime,mode=755 0 0
tmpfs /etc/cri/conf.d/01-registries.part tmpfs rw,seclabel,relatime,mode=755 0 0
tmpfs /etc/ssl/certs/ca-certificates tmpfs rw,seclabel,relatime,mode=755 0 0
tmpfs /etc/cri/conf.d/cri.toml tmpfs rw,seclabel,relatime,mode=755 0 0
tmpfs /etc/resolv.conf tmpfs rw,seclabel,relatime,mode=755 0 0
tmpfs /etc/hosts tmpfs rw,seclabel,relatime,mode=755 0 0
tmpfs /etc/cri/conf.d/base-spec.json tmpfs rw,seclabel,relatime,mode=755 0 0
tmpfs /etc/machine-id tmpfs rw,seclabel,relatime,mode=755 0 0
overlay /etc/cni overlay rw,seclabel,relatime,lowerdir=/etc/cni,upperdir=/var/system/overlays/etc-cni-diff,workdir=/var/system/overlays/etc-cni-workdir 0 0
overlay /etc/kubernetes overlay rw,seclabel,relatime,lowerdir=/etc/kubernetes,upperdir=/var/system/overlays/etc-kubernetes-diff,workdir=/var/system/overlays/etc-kubernetes-workdir 0 0

After:

❯ talosctl -n 10.5.0.3 read /proc/mounts | grep "/etc"
tmpfs /etc/cri/conf.d/hosts tmpfs ro,seclabel,relatime,mode=755 0 0
tmpfs /etc/cri/conf.d/01-registries.part tmpfs ro,seclabel,relatime,mode=755 0 0
tmpfs /etc/ssl/certs/ca-certificates tmpfs ro,seclabel,relatime,mode=755 0 0
tmpfs /etc/cri/conf.d/cri.toml tmpfs ro,seclabel,relatime,mode=755 0 0
tmpfs /etc/resolv.conf tmpfs ro,seclabel,relatime,mode=755 0 0
tmpfs /etc/hosts tmpfs ro,seclabel,relatime,mode=755 0 0
tmpfs /etc/cri/conf.d/base-spec.json tmpfs ro,seclabel,relatime,mode=755 0 0
tmpfs /etc/machine-id tmpfs ro,seclabel,relatime,mode=755 0 0
overlay /etc/cni overlay rw,seclabel,relatime,lowerdir=/etc/cni,upperdir=/var/system/overlays/etc-cni-diff,workdir=/var/system/overlays/etc-cni-workdir 0 0
overlay /etc/kubernetes overlay rw,seclabel,relatime,lowerdir=/etc/kubernetes,upperdir=/var/system/overlays/etc-kubernetes-diff,workdir=/var/system/overlays/etc-kubernetes-workdir 0 0

@@ -134,8 +134,8 @@ func (ctrl *EtcFileController) Run(ctx context.Context, r controller.Runtime, lo

logger.Debug("writing file contents", zap.String("dst", dst), zap.Stringer("version", spec.Metadata().Version()))

if err = UpdateFile(dst, spec.TypedSpec().Contents, spec.TypedSpec().Mode, spec.TypedSpec().SelinuxLabel); err != nil {
return fmt.Errorf("error updating %q: %w", dst, err)
if err = UpdateFile(src, spec.TypedSpec().Contents, spec.TypedSpec().Mode, spec.TypedSpec().SelinuxLabel); err != nil {
Copy link
Member Author

Choose a reason for hiding this comment

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

we were writing to /etc 💩

@frezbo frezbo force-pushed the fix/bind-mount-ro branch 3 times, most recently from 9c4995c to 0918906 Compare January 17, 2025 14:59
Fix the `/etc` bind mounts read-only by calling a remount as `ro`.

The proper fix is to use new `fsopen` and `fsmount` API's.

Signed-off-by: Noel Georgi <[email protected]>
@frezbo frezbo force-pushed the fix/bind-mount-ro branch from 0918906 to 8dea57a Compare January 17, 2025 15:05
@frezbo
Copy link
Member Author

frezbo commented Jan 17, 2025

/m

@talos-bot talos-bot merged commit 8dea57a into siderolabs:main Jan 17, 2025
50 checks passed
@frezbo frezbo deleted the fix/bind-mount-ro branch January 17, 2025 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Proposed
Status: Proposed
Development

Successfully merging this pull request may close these issues.

4 participants