-
Notifications
You must be signed in to change notification settings - Fork 18.9k
Allow for a read-only "/proc/sys/net". #47769
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
Conversation
d4b786d to
949486c
Compare
949486c to
6a26fbe
Compare
libnetwork/osl/namespace_linux.go
Outdated
| log.G(context.TODO()).WithError(err).Infof( | ||
| "Cannot disable IPv6 on container interface %s but DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE=1, continuing.", iface) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be useful to use a field for the interface for these logs? e.g. something like;
| log.G(context.TODO()).WithError(err).Infof( | |
| "Cannot disable IPv6 on container interface %s but DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE=1, continuing.", iface) | |
| log.G(context.TODO()).WithFields(log.Fields{ | |
| "error": err, | |
| "interface": iface, | |
| }).Info("Cannot disable IPv6 on container interface, but DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE=1, continuing") |
(same for the other logs changed in this PR)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done - I'm not sure how useful it is to include the interface name really, it'll probably be in err and it's a name we invented rather than something that'll be meaningful to the user. But, it was there before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, maybe it's not. My thinking of making it a structured field was that (imo) it's a good practice, and if we're consistent, it could allow for logs to be correlated ("here's logs related to interface=xxxxxx").
6a26fbe to
69f4dd2
Compare
libnetwork/osl/namespace_linux.go
Outdated
| logger.Warnf("Cannot enable IPv6 on container interface, continuing.") | ||
| } else if os.Getenv("DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE") == "1" { | ||
| // TODO(robmry) - remove this escape hatch for https://github.com/moby/moby/issues/47751 | ||
| // If the "/proc" file exists but isn't writable, we can't disable IPv6, which is | ||
| // https://github.com/moby/moby/security/advisories/GHSA-x84c-p2g9-rqv9 ... so, | ||
| // the user is required to override the error (or configure IPv6, or disable IPv6 | ||
| // by default in the OS, or make the "/proc" file writable). Once it's possible | ||
| // to enable IPv6 without having to configure IPAM etc, the env var should be | ||
| // removed. Then the user will have to explicitly enable IPv6 if it can't be | ||
| // disabled on the interface. | ||
| logger.Infof("Cannot disable IPv6 on container interface but DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE=1, continuing.") | ||
| } else { | ||
| logger.Errorf("Cannot disable IPv6 on container interface. Set env var DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE=1 to ignore.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit (not a blocker, but linters could start to warn about this); looks like we're no longer doing formatting so these could all be without f;
| logger.Warnf("Cannot enable IPv6 on container interface, continuing.") | |
| } else if os.Getenv("DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE") == "1" { | |
| // TODO(robmry) - remove this escape hatch for https://github.com/moby/moby/issues/47751 | |
| // If the "/proc" file exists but isn't writable, we can't disable IPv6, which is | |
| // https://github.com/moby/moby/security/advisories/GHSA-x84c-p2g9-rqv9 ... so, | |
| // the user is required to override the error (or configure IPv6, or disable IPv6 | |
| // by default in the OS, or make the "/proc" file writable). Once it's possible | |
| // to enable IPv6 without having to configure IPAM etc, the env var should be | |
| // removed. Then the user will have to explicitly enable IPv6 if it can't be | |
| // disabled on the interface. | |
| logger.Infof("Cannot disable IPv6 on container interface but DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE=1, continuing.") | |
| } else { | |
| logger.Errorf("Cannot disable IPv6 on container interface. Set env var DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE=1 to ignore.") | |
| logger.Warn("Cannot enable IPv6 on container interface, continuing.") | |
| } else if os.Getenv("DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE") == "1" { | |
| // TODO(robmry) - remove this escape hatch for https://github.com/moby/moby/issues/47751 | |
| // If the "/proc" file exists but isn't writable, we can't disable IPv6, which is | |
| // https://github.com/moby/moby/security/advisories/GHSA-x84c-p2g9-rqv9 ... so, | |
| // the user is required to override the error (or configure IPv6, or disable IPv6 | |
| // by default in the OS, or make the "/proc" file writable). Once it's possible | |
| // to enable IPv6 without having to configure IPAM etc, the env var should be | |
| // removed. Then the user will have to explicitly enable IPv6 if it can't be | |
| // disabled on the interface. | |
| logger.Info("Cannot disable IPv6 on container interface but DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE=1, continuing.") | |
| } else { | |
| logger.Error("Cannot disable IPv6 on container interface. Set env var DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE=1 to ignore.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, good point, thank you. I've fixed it.
thaJeztah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
left one nit, but not a blocker (so let me know if you want to update the PR before merging)
If dockerd runs on a host with a read-only /proc/sys/net filesystem, it isn't able to enable or disable IPv6 on network interfaces when attaching a container to a network (including initial networks during container creation). In release 26.0.2, a read-only /proc/sys/net meant container creation failed in all cases. So, don't attempt to enable/disable IPv6 on an interface if it's already set appropriately. If it's not possible to enable IPv6 when it's needed, just log (because that's what libnetwork has always done if IPv6 is disabled in the kernel). If it's not possible to disable IPv6 when it needs to be disabled, refuse to create the container and raise an error that suggests setting environment variable "DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE=1", to tell the daemon it's ok to ignore the problem. Signed-off-by: Rob Murray <rob.murray@docker.com>
69f4dd2 to
01ea18f
Compare
thaJeztah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
[](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [docker/docker](https://togithub.com/docker/docker) | patch | `26.1.0` -> `26.1.1` | --- ### Release Notes <details> <summary>docker/docker (docker/docker)</summary> ### [`v26.1.1`](https://togithub.com/moby/moby/releases/tag/v26.1.1) [Compare Source](https://togithub.com/docker/docker/compare/v26.1.0...v26.1.1) #### 26.1.1 For a full list of pull requests and changes in this release, refer to the relevant GitHub milestones: - [docker/cli, 26.1.1 milestone](https://togithub.com/docker/cli/issues?q=is%3Aclosed+milestone%3A26.1.1) - [moby/moby, 26.1.1 milestone](https://togithub.com/moby/moby/issues?q=is%3Aclosed+milestone%3A26.1.1) - Deprecated and removed features, see [Deprecated Features](https://togithub.com/docker/cli/blob/v26.1.1/docs/deprecated.md). - Changes to the Engine API, see [API version history](https://togithub.com/moby/moby/blob/v26.1.1/docs/api/version-history.md). ##### Bug fixes and enhancements - Fix `docker run -d` printing an `context canceled` spurious error when OTEL is configured. [docker/cli#5044](https://togithub.com/docker/cli/pull/5044) - Experimental environment variable `DOCKER_BRIDGE_PRESERVE_KERNEL_LL=1` will prevent the daemon from removing the kernel-assigned link local address on a Linux bridge. [moby/moby#47775](https://togithub.com/moby/moby/pull/47775) - Resolve an issue preventing container creation on hosts with a read-only `/proc/sys/net` filesystem. If IPv6 cannot be disabled on an interface due to this, either disable IPv6 by default on the host or ensure `/proc/sys/net` is read-write. Otherwise, start dockerd with `DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE=1` to bypass the error. [moby/moby#47769](https://togithub.com/moby/moby/pull/47769) > \[!NOTE] > The `DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE` is added as a temporary fix and will be phased out in a future major release after simplifying the IPv6 enablement process. ##### Packaging updates - Update BuildKit to [v0.13.2](https://togithub.com/moby/buildkit/releases/tag/v0.13.2). [moby/moby#47762](https://togithub.com/moby/moby/pull/47762) - Update Compose to [v2.27.0](https://togithub.com/docker/compose/releases/tag/v2.27.0). [docker/docker-ce-packages#1017](https://togithub.com/docker/docker-ce-packaging/pull/1017) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 6am on monday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/earthly/dind). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMzEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjMzMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZSJdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
[](https://renovatebot.com) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [docker/docker](https://togithub.com/docker/docker) | patch | `26.1.0` -> `26.1.1` | --- ### Release Notes <details> <summary>docker/docker (docker/docker)</summary> ### [`v26.1.1`](https://togithub.com/moby/moby/releases/tag/v26.1.1) [Compare Source](https://togithub.com/docker/docker/compare/v26.1.0...v26.1.1) #### 26.1.1 For a full list of pull requests and changes in this release, refer to the relevant GitHub milestones: - [docker/cli, 26.1.1 milestone](https://togithub.com/docker/cli/issues?q=is%3Aclosed+milestone%3A26.1.1) - [moby/moby, 26.1.1 milestone](https://togithub.com/moby/moby/issues?q=is%3Aclosed+milestone%3A26.1.1) - Deprecated and removed features, see [Deprecated Features](https://togithub.com/docker/cli/blob/v26.1.1/docs/deprecated.md). - Changes to the Engine API, see [API version history](https://togithub.com/moby/moby/blob/v26.1.1/docs/api/version-history.md). ##### Bug fixes and enhancements - Fix `docker run -d` printing an `context canceled` spurious error when OTEL is configured. [docker/cli#5044](https://togithub.com/docker/cli/pull/5044) - Experimental environment variable `DOCKER_BRIDGE_PRESERVE_KERNEL_LL=1` will prevent the daemon from removing the kernel-assigned link local address on a Linux bridge. [moby/moby#47775](https://togithub.com/moby/moby/pull/47775) - Resolve an issue preventing container creation on hosts with a read-only `/proc/sys/net` filesystem. If IPv6 cannot be disabled on an interface due to this, either disable IPv6 by default on the host or ensure `/proc/sys/net` is read-write. Otherwise, start dockerd with `DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE=1` to bypass the error. [moby/moby#47769](https://togithub.com/moby/moby/pull/47769) > \[!NOTE] > The `DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE` is added as a temporary fix and will be phased out in a future major release after simplifying the IPv6 enablement process. ##### Packaging updates - Update BuildKit to [v0.13.2](https://togithub.com/moby/buildkit/releases/tag/v0.13.2). [moby/moby#47762](https://togithub.com/moby/moby/pull/47762) - Update Compose to [v2.27.0](https://togithub.com/docker/compose/releases/tag/v2.27.0). [docker/docker-ce-packages#1017](https://togithub.com/docker/docker-ce-packaging/pull/1017) </details> --- ### Configuration 📅 **Schedule**: Branch creation - "after 6am on monday" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/earthly/dind). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zMzEuMCIsInVwZGF0ZWRJblZlciI6IjM3LjMzMS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZSJdfQ==--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Works around failure of Docker to create containers (moby/moby#47769)— level=error msg="Cannot disable IPv6 on container interface." error="open /proc/sys/net/ipv6/conf/eth0/disable_ipv6: read-only file system" interface=eth0 —as suggested in the Docker Engine 27.0.1 release notes.
- What I did
If dockerd runs on a host with a read-only
/proc/sys/netfilesystem, it isn't able to enable or disable IPv6 on network interfaces when attaching a container to a network (including initial networks during container creation).In release 26.0.2, a read-only
/proc/sys/netmeant container creation failed in all cases. Now, thedisable_ipv6setting is only modified if necessary and, if IPv6 can't be disabled when it needs to be, an environment variable can be used to tell dockerd to proceed anyway.Fixes #47751
The plan is to remove the environment variable once it's easier to enable IPv6, probably in release 27.0 - at that point, on a system where IPv6 can't be disabled on an interface, IPv6 will have to be explicitly allowed in the network configuration. #47773.
- How I did it
Don't attempt to enable/disable IPv6 on an interface if it's already set appropriately.
If it's not possible to enable IPv6 when it's needed, just log (because that's what libnetwork has always done if IPv6 is disabled in the kernel).
If it's not possible to disable IPv6 when it needs to be disabled, refuse to create the container and raise an error that suggests setting environment variable "DOCKER_ALLOW_IPV6_ON_IPV4_INTERFACE=1", to tell the daemon it's ok to ignore the problem.
- How to verify it
New regression test - environment variable
DOCKER_TEST_RO_DISABLE_IPV6is used to simulate failure to modify the IPv6 setting.- Description for the changelog