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
Allow for a read-only "/proc/sys/net". #47769
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.
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
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
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](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>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](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>
- What I did
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. Now, thedisable_ipv6
setting 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_IPV6
is used to simulate failure to modify the IPv6 setting.- Description for the changelog