Skip to content

Be aware of security hardened mount points#1340

Merged
debarshiray merged 3 commits intocontainers:mainfrom
debarshiray:wip/rishi/issue-911
Aug 12, 2023
Merged

Be aware of security hardened mount points#1340
debarshiray merged 3 commits intocontainers:mainfrom
debarshiray:wip/rishi/issue-911

Conversation

@debarshiray
Copy link
Copy Markdown
Member

@debarshiray debarshiray commented Jul 13, 2023

Sometimes locations such as /var/lib/flatpak, /var/lib/systemd/coredump and /var/log/journal sit on security hardened mount points that are marked as nosuid,nodev,noexec [1]. In such cases, when Toolbx is used rootless, an attempt to bind mount these locations read-only at runtime with mount(8) fails because of permission problems:

  # mount --rbind -o ro <source> <containerPath>
  mount: <containerPath>: filesystem was mounted, but any subsequent
      operation failed: Unknown error 5005.

(Note that the above error message from mount(8) was subsequently improved to show something more meaningful than 'Unknown error' [2].)

The problem is that init-container is running inside the container's mount and user namespace and the source paths were mounted inside the host's namespace with nosuid,nodev,noexec. The above mount(8) call tries to remove the nosuid,nodev,noexec flags from the mount point and replace them with only ro, which is something that can't be done from a child namespace.

Note that this doesn't fail when Toolbx is running as root. This is because the container uses the host's user namespace and is able to remove the nosuid,nodev,noexec flags from the mount point and replace them with only ro. Even though it doesn't fail, the flags shouldn't get replaced like that inside the container, because it removes the security hardening of those mount points.

There's actually no benefit in bind mounting these paths as read-only. It was historically done this way 'just to be safe' because a user isn't expected to write to these locations from inside a container. However, Toolbx doesn't intend to provide any heightened security beyond what's already available on the host.

Hence, it's better to get out of the way and leave it to the permissions on the source location from the host operating system to guard the castle. This is accomplished by not passing any file system options to mount(8) [1].

Note that this isn't a problem when Toolbx is running as root, because the container uses the host's user namespace.

Based on an idea from Si.

[1] https://man7.org/linux/man-pages/man8/mount.8.html

[2] util-linux commit 9420ca34dc8b6f0f
util-linux/util-linux@9420ca34dc8b6f0f
util-linux/util-linux#2376

#911

@debarshiray debarshiray requested a review from martymichal as a code owner July 13, 2023 13:06
@softwarefactory-project-zuul
Copy link
Copy Markdown

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/917913a4bcb8443fb1fe8d10efe0c9b9

✔️ unit-test SUCCESS in 9m 43s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 26s
✔️ unit-test-restricted SUCCESS in 8m 22s
✔️ system-test-fedora-rawhide SUCCESS in 25m 31s
✔️ system-test-fedora-38 SUCCESS in 26m 55s
✔️ system-test-fedora-37 SUCCESS in 26m 26s

@debarshiray debarshiray changed the title cmd/initContainer: Handle security-hardened mount points when rootless cmd/initContainer: Handle security hardened mount points when rootless Jul 13, 2023
@debarshiray debarshiray force-pushed the wip/rishi/issue-911 branch from a71e0f5 to 65afe2f Compare July 13, 2023 13:37
@softwarefactory-project-zuul
Copy link
Copy Markdown

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/e59a0c1539734b2bb72e034eb26e73b5

✔️ unit-test SUCCESS in 9m 42s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 05s
✔️ unit-test-restricted SUCCESS in 8m 40s
✔️ system-test-fedora-rawhide SUCCESS in 28m 12s
✔️ system-test-fedora-38 SUCCESS in 27m 14s
✔️ system-test-fedora-37 SUCCESS in 26m 57s

Otherwise https://www.shellcheck.net/ would complain:
  Line 218:
  source <(echo "$output")
         ^---------------^ SC1090 (warning): ShellCheck can't follow
                           non-constant source. Use a directive to
                           specify location.

See: https://www.shellcheck.net/wiki/SC1090

containers#1347
Otherwise https://www.shellcheck.net/ would complain
  Line 110:
  for ((i = ${num_of_retries}; i > 0; i--)); do
            ^---------------^ SC2004 (style): $/${} is unnecessary on
                              arithmetic variables.

See: https://www.shellcheck.net/wiki/SC2004

containers#1347
Sometimes locations such as /var/lib/flatpak, /var/lib/systemd/coredump
and /var/log/journal sit on security hardened mount points that are
marked as 'nosuid,nodev,noexec' [1].  In such cases, when Toolbx is used
rootless, an attempt to bind mount these locations read-only at runtime
with mount(8) fails because of permission problems:
  # mount --rbind -o ro <source> <containerPath>
  mount: <containerPath>: filesystem was mounted, but any subsequent
      operation failed: Unknown error 5005.

(Note that the above error message from mount(8) was subsequently
improved to show something more meaningful than 'Unknown error' [2].)

The problem is that 'init-container' is running inside the container's
mount and user namespace, and the source paths were mounted inside the
host's namespace with 'nosuid,nodev,noexec'.  The above mount(8) call
tries to remove the 'nosuid,nodev,noexec' flags from the mount point and
replace them with only 'ro', which is something that can't be done from
a child namespace.

Note that this doesn't fail when Toolbx is running as root.  This is
because the container uses the host's user namespace and is able to
remove the 'nosuid,nodev,noexec' flags from the mount point and replace
them with only 'ro'.  Even though it doesn't fail, the flags shouldn't
get replaced like that inside the container, because it removes the
security hardening of those mount points.

There's actually no benefit in bind mounting these paths as read-only.
It was historically done this way 'just to be safe' because a user isn't
expected to write to these locations from inside a container.  However,
Toolbx doesn't intend to provide any heightened security beyond what's
already available on the host.

Hence, it's better to get out of the way and leave it to the permissions
on the source location from the host operating system to guard the
castle.  This is accomplished by not passing any file system options to
mount(8) [1].

Based on an idea from Si.

[1] https://man7.org/linux/man-pages/man8/mount.8.html

[2] util-linux commit 9420ca34dc8b6f0f
    util-linux/util-linux@9420ca34dc8b6f0f
    util-linux/util-linux#2376

containers#911
@debarshiray debarshiray changed the title cmd/initContainer: Handle security hardened mount points when rootless Be aware of security hardened mount points Aug 11, 2023
@softwarefactory-project-zuul
Copy link
Copy Markdown

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/4e335f18da584a65b912e799935dedfe

unit-test RETRY_LIMIT in 32s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 32s
unit-test-restricted RETRY_LIMIT in 34s
system-test-fedora-rawhide RETRY_LIMIT in 36s
✔️ system-test-fedora-38 SUCCESS in 29m 17s
✔️ system-test-fedora-37 SUCCESS in 27m 13s

@debarshiray
Copy link
Copy Markdown
Member Author

The tests run on Fedora Rawhide nodes are failing because of the same reasons as in #1344 and #1331 , and the root cause appears to be rsync: https://bugzilla.redhat.com/show_bug.cgi?id=2229654

So, I am going to temporarily ignore these test failures on Fedora Rawhide.

@debarshiray debarshiray merged commit 1cc9e07 into containers:main Aug 12, 2023
@debarshiray debarshiray deleted the wip/rishi/issue-911 branch August 12, 2023 10:41
debarshiray pushed a commit to alatiera/toolbox that referenced this pull request Aug 22, 2023
On new builds of GNOME OS [1], the host's / is mounted with 'nodev,...'
and those flags are also inherited by /etc because it's not a separate
mount point.  This leads to the same problem with /etc/machine-id that
was seen before with /var/lib/flatpak, /var/lib/systemd/coredump and
/var/log/journal [2].

Therefore, use the same approach [2] to handle /etc/machine-id.

[1] https://gitlab.gnome.org/GNOME/gnome-build-meta/-/issues/718

[2] Commit 1cc9e07
    containers@1cc9e07b7c36fe9f
    containers#1340

containers#911

Signed-off-by: Jordan Petridis <jordan@centricular.com>
debarshiray pushed a commit to alatiera/toolbox that referenced this pull request Aug 22, 2023
On new builds of GNOME OS [1], the host's / is mounted with 'nodev,...'
and those flags are also inherited by /etc because it's not a separate
mount point.  This leads to the same problem with /etc/machine-id that
was seen before with /var/lib/flatpak, /var/lib/systemd/coredump and
/var/log/journal [2].

Therefore, use the same approach [2] to handle /etc/machine-id.

[1] https://gitlab.gnome.org/GNOME/gnome-build-meta/-/issues/718

[2] Commit 1cc9e07
    containers@1cc9e07b7c36fe9f
    containers#1340

containers#911
containers#1354

Signed-off-by: Jordan Petridis <jordan@centricular.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant