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

Always configure iptables forward policy #2450

Merged
merged 1 commit into from
Sep 17, 2019
Merged

Conversation

TheNodi
Copy link
Contributor

@TheNodi TheNodi commented Sep 12, 2019

Follow-up email to security@docker.com, ping @justincormack
Related to moby/moby#14041 and #1526

By default, when Docker starts up (to configure the bridge network driver) it enables ip_forward flag and it sets iptables FORWARD chain policy to DROP (unless daemon's iptables option is set to false). Then Docker inserts ACCEPT rules based on the configured networks.

If another software (or an administrator) enables ip_forward flag before Docker boot, iptables policy configuration is skipped, resulting in the default policy being ACCEPT. Docker still configures all other rules, but they have no meaning because they are only ACCEPT rules (and the default is ACCEPT anyway). These lead to a local attacker being able to access all containers by setting the host as gateway for the (guessable) Docker subnets (as shown in moby/moby#14041).

Accessing all containers can be a security vulnerability, especially when unauthenticated private services are running (e.g., Redis). An example software that interferes with Docker is OpenVPN, because it enables ip_forward flag at boot before Docker has a change of reading it. OpenVPN has the increased problem of allowing all clients connected to the vpn server the ability of abusing this vulnerability acting as local nodes in the network (I've found this scenario multiple times in the wild), but it is not the only software causing this issue.
To reproduce it, you can use a fresh copy of Ubuntu 18.04, install Docker and then install OpenVPN (Official APT repository, no need to configure VPN server/clients) and reboot the server. On next boot, iptables forward policy remains ACCEPT.

Because Docker's iptables flag is still true, and because Docker is still configuring all other iptables' rules, I think it should configure iptables forward policy even when ip_forward flag is 1. This would ensure a secure setup by default, and if the user wants more control over iptables, he should explicitly disable Docker's iptables management.

@GordonTheTurtle
Copy link

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "iptables-policy" git@github.com:TheNodi/libnetwork.git somewhere
$ cd somewhere
$ git commit --amend -s --no-edit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

Signed-off-by: Leonardo Nodari <me@leonardonodari.it>
@justincormack
Copy link
Contributor

cc @arkodg @tiborvass

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TheNodi thanks for raising this issue and addressing it.

Sidenote - There's going to be a tiny percent of folks who will resent this override :)

@arkodg
Copy link
Contributor

arkodg commented Sep 16, 2019

@euanh @selansen can we merge this ?

@euanh euanh merged commit 96bcc0d into moby:master Sep 17, 2019
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Sep 20, 2019
full diff: moby/libnetwork@92d1fbe...96bcc0d

changes included:

- moby/libnetwork#2429 Updating IPAM config with results from HNS create network call
  - addresses moby#38358
- moby/libnetwork#2450 Always configure iptables forward policy
  - related to moby#14041 and moby/libnetwork#1526

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-jenkins pushed a commit to docker/docker-ce that referenced this pull request Sep 23, 2019
full diff: moby/libnetwork@92d1fbe...96bcc0d

changes included:

- moby/libnetwork#2429 Updating IPAM config with results from HNS create network call
  - addresses moby/moby#38358
- moby/libnetwork#2450 Always configure iptables forward policy
  - related to moby/moby#14041 and moby/libnetwork#1526

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 75477f0b3c77f2108a6b5586dbc246c52b479941
Component: engine
thaJeztah added a commit to thaJeztah/docker that referenced this pull request Sep 24, 2019
full diff: moby/libnetwork@92d1fbe...96bcc0d

changes included:

- moby/libnetwork#2429 Updating IPAM config with results from HNS create network call
  - addresses moby#38358
- moby/libnetwork#2450 Always configure iptables forward policy
  - related to moby#14041 and moby/libnetwork#1526

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 75477f0)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
docker-jenkins pushed a commit to docker/docker-ce that referenced this pull request Sep 25, 2019
full diff: moby/libnetwork@92d1fbe...96bcc0d

changes included:

- moby/libnetwork#2429 Updating IPAM config with results from HNS create network call
  - addresses moby/moby#38358
- moby/libnetwork#2450 Always configure iptables forward policy
  - related to moby/moby#14041 and moby/libnetwork#1526

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 75477f0b3c77f2108a6b5586dbc246c52b479941)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: 559be42fc26048f4069de64f84202803a113413a
Component: engine
@euanh
Copy link
Collaborator

euanh commented Oct 7, 2019

Related: moby/moby#39439

thaJeztah added a commit to thaJeztah/docker that referenced this pull request Oct 7, 2019
The patch made in  moby/libnetwork#2450 caused a breaking change in the
networking behaviour, causing Kubernetes installations on Docker Desktop
(and possibly other setups) to fail.

Rolling back this change in the 19.03 branch while we investigate if there
are alternatives.

diff: moby/libnetwork@45c7102...96bcc0d

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
euanh added a commit that referenced this pull request Oct 7, 2019
Reverts 96bcc0d (PR #2450)

Fallout from changing the forwarding default policy to deny was greater than anticipated.

Signed-off-by: Euan Harris <euan.harris@docker.com>
docker-jenkins pushed a commit to docker/docker-ce that referenced this pull request Oct 8, 2019
The patch made in  moby/libnetwork#2450 caused a breaking change in the
networking behaviour, causing Kubernetes installations on Docker Desktop
(and possibly other setups) to fail.

Rolling back this change in the 19.03 branch while we investigate if there
are alternatives.

diff: moby/libnetwork@45c7102...96bcc0d

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Upstream-commit: fb0fca86077528466cc66fef20697537941ca125
Component: engine
@thaJeztah thaJeztah mentioned this pull request Oct 11, 2019
burnMyDread pushed a commit to burnMyDread/moby that referenced this pull request Oct 21, 2019
full diff: moby/libnetwork@92d1fbe...96bcc0d

changes included:

- moby/libnetwork#2429 Updating IPAM config with results from HNS create network call
  - addresses moby#38358
- moby/libnetwork#2450 Always configure iptables forward policy
  - related to moby#14041 and moby/libnetwork#1526

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: zach <Zachary.Joyner@linux.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.

None yet

6 participants