From fde80fe2e731f04ce029dc93fcdadeccabe2921b Mon Sep 17 00:00:00 2001 From: Rob Murray Date: Sat, 23 Mar 2024 16:49:53 +0000 Subject: [PATCH] Restore the SetKey prestart hook. Partially reverts 0046b16 "daemon: set libnetwork sandbox key w/o OCI hook" Running SetKey to store the OCI Sandbox key after task creation, rather than from the OCI prestart hook, meant it happened after sysctl settings were applied by the runtime - which was the intention, we wanted to complete Sandbox configuration after IPv6 had been disabled by a sysctl if that was going to happen. But, it meant '--sysctl' options for a specfic network interface caused container task creation to fail, because the interface is only moved into the network namespace during SetKey. This change restores the SetKey prestart hook, and regenerates config files that depend on the container's support for IPv6 after the task has been created. It also adds a regression test that makes sure it's possible to set an interface-specfic sysctl. Signed-off-by: Rob Murray --- daemon/oci_linux.go | 24 ++++++++++++++++ daemon/start_linux.go | 8 ++---- integration/networking/bridge_test.go | 28 +++++++++++++++++++ libnetwork/osl/namespace_linux.go | 40 +++++++++++++++++---------- libnetwork/sandbox_linux.go | 40 +++++++++++++++++++-------- 5 files changed, 108 insertions(+), 32 deletions(-) diff --git a/daemon/oci_linux.go b/daemon/oci_linux.go index 276d6abe95..37ad941847 100644 --- a/daemon/oci_linux.go +++ b/daemon/oci_linux.go @@ -24,6 +24,7 @@ import ( "github.com/docker/docker/oci/caps" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/rootless/specconv" + "github.com/docker/docker/pkg/stringid" volumemounts "github.com/docker/docker/volume/mounts" "github.com/moby/sys/mount" "github.com/moby/sys/mountinfo" @@ -60,6 +61,28 @@ func withRlimits(daemon *Daemon, daemonCfg *dconfig.Config, c *container.Contain } } +// withLibnetwork sets the libnetwork hook +func withLibnetwork(daemon *Daemon, daemonCfg *dconfig.Config, c *container.Container) coci.SpecOpts { + return func(ctx context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error { + if c.Config.NetworkDisabled { + return nil + } + for _, ns := range s.Linux.Namespaces { + if ns.Type == specs.NetworkNamespace && ns.Path == "" { + if s.Hooks == nil { + s.Hooks = &specs.Hooks{} + } + shortNetCtlrID := stringid.TruncateID(daemon.netController.ID()) + s.Hooks.Prestart = append(s.Hooks.Prestart, specs.Hook{ + Path: filepath.Join("/proc", strconv.Itoa(os.Getpid()), "exe"), + Args: []string{"libnetwork-setkey", "-exec-root=" + daemonCfg.GetExecRoot(), c.ID, shortNetCtlrID}, + }) + } + } + return nil + } +} + // withRootless sets the spec to the rootless configuration func withRootless(daemon *Daemon, daemonCfg *dconfig.Config) coci.SpecOpts { return func(_ context.Context, _ coci.Client, _ *containers.Container, s *coci.Spec) error { @@ -1015,6 +1038,7 @@ func (daemon *Daemon) createSpec(ctx context.Context, daemonCfg *configStore, c WithCapabilities(c), WithSeccomp(daemon, c), withMounts(daemon, daemonCfg, c, mounts), + withLibnetwork(daemon, &daemonCfg.Config, c), WithApparmor(c), WithSelinux(c), WithOOMScore(&c.HostConfig.OomScoreAdj), diff --git a/daemon/start_linux.go b/daemon/start_linux.go index f4c0044dbf..72e6b2e48a 100644 --- a/daemon/start_linux.go +++ b/daemon/start_linux.go @@ -2,14 +2,12 @@ package daemon // import "github.com/docker/docker/daemon" import ( "context" - "fmt" - - specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/docker/docker/container" "github.com/docker/docker/errdefs" "github.com/docker/docker/libcontainerd/types" "github.com/docker/docker/oci" + specs "github.com/opencontainers/runtime-spec/specs-go" ) // initializeCreatedTask performs any initialization that needs to be done to @@ -22,9 +20,7 @@ func (daemon *Daemon) initializeCreatedTask(ctx context.Context, tsk types.Task, if err != nil { return errdefs.System(err) } - if err := sb.SetKey(fmt.Sprintf("/proc/%d/ns/net", tsk.Pid())); err != nil { - return errdefs.System(err) - } + return sb.FinishConfig() } } return nil diff --git a/integration/networking/bridge_test.go b/integration/networking/bridge_test.go index 7ac40c8d48..b5eb0fdce0 100644 --- a/integration/networking/bridge_test.go +++ b/integration/networking/bridge_test.go @@ -675,3 +675,31 @@ func TestDisableIPv6Addrs(t *testing.T) { }) } } + +// Test that it's possible to set a sysctl on an interface in the container. +// Regression test for https://github.com/moby/moby/issues/47619 +func TestSetInterfaceSysctl(t *testing.T) { + skip.If(t, testEnv.DaemonInfo.OSType == "windows", "no sysctl on Windows") + + ctx := setupTest(t) + d := daemon.New(t) + d.StartWithBusybox(ctx, t) + defer d.Stop(t) + + c := d.NewClientT(t) + defer c.Close() + + const scName = "net.ipv4.conf.eth0.forwarding" + opts := []func(config *container.TestContainerConfig){ + container.WithCmd("sysctl", scName), + container.WithSysctls(map[string]string{scName: "1"}), + } + + runRes := container.RunAttach(ctx, t, c, opts...) + defer c.ContainerRemove(ctx, runRes.ContainerID, + containertypes.RemoveOptions{Force: true}, + ) + + stdout := runRes.Stdout.String() + assert.Check(t, is.Contains(stdout, scName)) +} diff --git a/libnetwork/osl/namespace_linux.go b/libnetwork/osl/namespace_linux.go index fb6cd13492..8a41f6aa22 100644 --- a/libnetwork/osl/namespace_linux.go +++ b/libnetwork/osl/namespace_linux.go @@ -545,26 +545,38 @@ func (n *Namespace) Restore(interfaces map[Iface][]IfaceOption, routes []*types. return nil } -// IPv6LoEnabled checks whether the loopback interface has an IPv6 address ('::1' -// is assigned by the kernel if IPv6 is enabled). +// IPv6LoEnabled returns true if the loopback interface had an IPv6 address when +// last checked. It's always checked on the first call, and by RefreshIPv6LoEnabled. +// ('::1' is assigned by the kernel if IPv6 is enabled.) func (n *Namespace) IPv6LoEnabled() bool { n.ipv6LoEnabledOnce.Do(func() { - // If anything goes wrong, assume no-IPv6. - iface, err := n.nlHandle.LinkByName("lo") - if err != nil { - log.G(context.TODO()).WithError(err).Warn("Unable to find 'lo' to determine IPv6 support") - return - } - addrs, err := n.nlHandle.AddrList(iface, nl.FAMILY_V6) - if err != nil { - log.G(context.TODO()).WithError(err).Warn("Unable to get 'lo' addresses to determine IPv6 support") - return - } - n.ipv6LoEnabledCached = len(addrs) > 0 + n.RefreshIPv6LoEnabled() }) + n.mu.Lock() + defer n.mu.Unlock() return n.ipv6LoEnabledCached } +// RefreshIPv6LoEnabled refreshes the cached result returned by IPv6LoEnabled. +func (n *Namespace) RefreshIPv6LoEnabled() { + n.mu.Lock() + defer n.mu.Unlock() + + // If anything goes wrong, assume no-IPv6. + n.ipv6LoEnabledCached = false + iface, err := n.nlHandle.LinkByName("lo") + if err != nil { + log.G(context.TODO()).WithError(err).Warn("Unable to find 'lo' to determine IPv6 support") + return + } + addrs, err := n.nlHandle.AddrList(iface, nl.FAMILY_V6) + if err != nil { + log.G(context.TODO()).WithError(err).Warn("Unable to get 'lo' addresses to determine IPv6 support") + return + } + n.ipv6LoEnabledCached = len(addrs) > 0 +} + // ApplyOSTweaks applies operating system specific knobs on the sandbox. func (n *Namespace) ApplyOSTweaks(types []SandboxType) { for _, t := range types { diff --git a/libnetwork/sandbox_linux.go b/libnetwork/sandbox_linux.go index 9f3c057bab..7db5edf7c6 100644 --- a/libnetwork/sandbox_linux.go +++ b/libnetwork/sandbox_linux.go @@ -90,12 +90,8 @@ func (sb *Sandbox) updateGateway(ep *Endpoint) error { return fmt.Errorf("failed to set gateway while updating gateway: %v", err) } - // If IPv6 has been disabled in the sandbox a gateway may still have been - // configured, don't attempt to apply it. - if ipv6, ok := sb.ipv6Enabled(); !ok || ipv6 { - if err := osSbox.SetGatewayIPv6(joinInfo.gw6); err != nil { - return fmt.Errorf("failed to set IPv6 gateway while updating gateway: %v", err) - } + if err := osSbox.SetGatewayIPv6(joinInfo.gw6); err != nil { + return fmt.Errorf("failed to set IPv6 gateway while updating gateway: %v", err) } return nil @@ -162,6 +158,10 @@ func (sb *Sandbox) SetKey(basePath string) error { } } + // Set up hosts and resolv.conf files. IPv6 support in the container can't be + // determined yet, as sysctls haven't been applied by the runtime. Calling + // FinishInit after the container task has been created, when sysctls have been + // applied will regenerate these files. if err := sb.finishInitDNS(); err != nil { return err } @@ -175,6 +175,27 @@ func (sb *Sandbox) SetKey(basePath string) error { return nil } +// FinishConfig completes Sandbox configuration. If called after the container task has been +// created, and sysctl settings applied, the configuration will be based on the container's +// IPv6 support. +func (sb *Sandbox) FinishConfig() error { + if sb.config.useDefaultSandBox { + return nil + } + + sb.mu.Lock() + osSbox := sb.osSbox + sb.mu.Unlock() + if osSbox == nil { + return nil + } + + // If sysctl changes have been made, IPv6 may have been enabled/disabled since last checked. + osSbox.RefreshIPv6LoEnabled() + + return sb.finishInitDNS() +} + // IPv6 support can always be determined for host networking. For other network // types it can only be determined once there's a container namespace to probe, // return ok=false in that case. @@ -283,12 +304,7 @@ func (sb *Sandbox) populateNetworkResources(ep *Endpoint) error { ifaceOptions = append(ifaceOptions, osl.WithIPv4Address(i.addr), osl.WithRoutes(i.routes)) if i.addrv6 != nil && i.addrv6.IP.To16() != nil { - // If IPv6 has been disabled in the Sandbox, an IPv6 address will still have - // been allocated. Don't apply it, because doing so would enable IPv6 on the - // interface. - if ipv6, ok := sb.ipv6Enabled(); !ok || ipv6 { - ifaceOptions = append(ifaceOptions, osl.WithIPv6Address(i.addrv6)) - } + ifaceOptions = append(ifaceOptions, osl.WithIPv6Address(i.addrv6)) } if len(i.llAddrs) != 0 { ifaceOptions = append(ifaceOptions, osl.WithLinkLocalAddresses(i.llAddrs))