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

[20.10 backport] daemon.WithCommonOptions() fix detection of user-namespaces #43084

Merged
merged 1 commit into from Jan 8, 2022

Conversation

AkihiroSuda
Copy link
Member

@AkihiroSuda AkihiroSuda commented Dec 15, 2021

Cherry-pick #42736

Commit dae652e (#41030) added support for non-privileged containers to use ICMP_PROTO (used for ping). This option cannot be set for containers that have user-namespaces enabled.

However, the detection looks to be incorrect; HostConfig.UsernsMode was added in 6993e89 (#20111) / ee21838 (#20913), and the property only has meaning if the daemon is running with user namespaces enabled. In other situations, the property has no meaning.

As a result of the above, the sysctl would only be set for containers running with UsernsMode=host on a daemon running with user-namespaces enabled.

This patch adds a check if the daemon has user-namespaces enabled (RemappedRoot having a non-empty value) to fix the detection.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

The cherry-pick was almost clean but userns.RunningInUserNS() -> sys.RunningInUserNS().

diff --git a/daemon/oci_linux.go b/daemon/oci_linux.go
index 9b39b8f615..3d34dcb31d 100644
--- a/daemon/oci_linux.go
+++ b/daemon/oci_linux.go
@@ -768,7 +768,7 @@ func WithCommonOptions(daemon *Daemon, c *container.Container) coci.SpecOpts {
                if c.HostConfig.NetworkMode.IsPrivate() {
                        // We cannot set up ping socket support in a user namespace
                        userNS := daemon.configStore.RemappedRoot != "" && c.HostConfig.UsernsMode.IsPrivate()
-                       if !userNS && !userns.RunningInUserNS() && sysctlExists("net.ipv4.ping_group_range") {
+                       if !userNS && !sys.RunningInUserNS() && sysctlExists("net.ipv4.ping_group_range") {
                                // allow unprivileged ICMP echo sockets without CAP_NET_RAW
                                s.Linux.Sysctl["net.ipv4.ping_group_range"] = "0 2147483647"
                        }

Fix docker/buildx#561

Commit dae652e added support for non-privileged
containers to use ICMP_PROTO (used for `ping`). This option cannot be set for
containers that have user-namespaces enabled.

However, the detection looks to be incorrect; HostConfig.UsernsMode was added
in 6993e89 / ee21838,
and the property only has meaning if the daemon is running with user namespaces
enabled. In other situations, the property has no meaning.
As a result of the above, the sysctl would only be set for containers running
with UsernsMode=host on a daemon running with user-namespaces enabled.

This patch adds a check if the daemon has user-namespaces enabled (RemappedRoot
having a non-empty value), or if the daemon is running inside a user namespace
(e.g. rootless mode) to fix the detection.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit a826ca3)

---
The cherry-pick was almost clean but `userns.RunningInUserNS()` -> `sys.RunningInUserNS()`.

Fix docker/buildx issue 561
---

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
@AkihiroSuda AkihiroSuda changed the title daemon.WithCommonOptions() fix detection of user-namespaces [20.10 backport] daemon.WithCommonOptions() fix detection of user-namespaces Dec 15, 2021
@AkihiroSuda AkihiroSuda added this to the 20.10.13 milestone Dec 15, 2021
Copy link
Member

@thaJeztah thaJeztah left a comment

LGTM

@AkihiroSuda AkihiroSuda requested a review from cpuguy83 Jan 7, 2022
tianon
tianon approved these changes Jan 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants