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

Attempt to add static iptables rules for DOCKER-USER on CentOS 7 #8357

Closed
wants to merge 14 commits into from

Conversation

paigehargrave
Copy link
Contributor

#8087

Extra information (is any of this is useful):

$ docker run -d -p 7777:6379 --name data1 redis
$ docker run -d -p 8888:6379 --name data2 redis
$ sudo iptables -N DOCKER-USER-redis1
$ sudo iptables -A DOCKER-USER-redis1 -s 192.168.56.0/24 -p tcp -m tcp -j RETURN
$ sudo iptables -A DOCKER-USER-redis1 -j REJECT --reject-with icmp-port-unreachable
$ sudo iptables -N DOCKER-USER-redis2
$ sudo iptables -A DOCKER-USER-redis2 -s 10.0.24.0/24 -p tcp -m tcp -j RETURN
$ sudo iptables -A DOCKER-USER-redis2 -j REJECT --reject-with icmp-port-unreachable
$ sudo iptables -A DOCKER-USER -i eth0 -p tcp -m conntrack --ctorigdstport 7777 -j DOCKER-USER-redis1
$ sudo iptables -A DOCKER-USER -i eth0 -p tcp -m conntrack --ctorigdstport 8888 -j DOCKER-USER-redis2

"I think an example like this belongs in the docs as it probably covers what 99% of users are looking for: the ability to expose ports using -p but still be able to control traffic to them using common filters like -s."

Note that --ctorigdstport matches the original destination port of the first packet of the connection, not the packet being filtered. So the dropping rule will also drop responses to the outgoing connections from Docker to the world on 5000-9999! Add --ctdir ORIGINAL to the DROP rule to match only incoming packets. See github.com/moby/moby/issues/22054#issuecomment-466663033

You can also specify which chains docker should use. For example, in the filter table, specify another chain instead of FORWARD. This allows you to use traditional tools to manage the firewall and decide when to pass control to docker.

Information pulled from:
moby/moby#33567
https://unrouted.io/2017/08/15/docker-firewall/
moby/libnetwork#1675

Extra information:

$ docker run -d -p 7777:6379 --name data1 redis
$ docker run -d -p 8888:6379 --name data2 redis
$ sudo iptables -N DOCKER-USER-redis1
$ sudo iptables -A DOCKER-USER-redis1 -s 192.168.56.0/24 -p tcp -m tcp -j RETURN
$ sudo iptables -A DOCKER-USER-redis1 -j REJECT --reject-with icmp-port-unreachable
$ sudo iptables -N DOCKER-USER-redis2
$ sudo iptables -A DOCKER-USER-redis2 -s 10.0.24.0/24 -p tcp -m tcp -j RETURN
$ sudo iptables -A DOCKER-USER-redis2 -j REJECT --reject-with icmp-port-unreachable
$ sudo iptables -A DOCKER-USER -i eth0 -p tcp -m conntrack --ctorigdstport 7777 -j DOCKER-USER-redis1
$ sudo iptables -A DOCKER-USER -i eth0 -p tcp -m conntrack --ctorigdstport 8888 -j DOCKER-USER-redis2

"I think an example like this belongs in the docs as it probably covers what 99% of users are looking for: the ability to expose ports using `-p` but still be able to control traffic to them using common filters like `-s`."


Note that --ctorigdstport matches the original destination port of the first packet of the connection, not the packet being filtered. So the dropping rule will also drop responses to the outgoing connections from Docker to the world on 5000-9999! Add --ctdir ORIGINAL to the DROP rule to match only incoming packets. See github.com/moby/moby/issues/22054#issuecomment-466663033

You can also specify which chains docker should use. For example, in the filter table, specify another chain instead of `FORWARD`. This allows you to use traditional tools to manage the firewall and decide when to pass control to docker.

Information pulled from:
moby/moby#33567
https://unrouted.io/2017/08/15/docker-firewall/
moby/libnetwork#1675
@paigehargrave
Copy link
Contributor Author

@thaJeztah Hi Sebastiaan - There was a lot of information for this - not knowing the actual steps here, I tried to cobble together some information. PTAL - I'm guessing I've got some gaps. Thank you in advance!

@GordonTheTurtle
Copy link

GordonTheTurtle commented Feb 26, 2019

Deploy preview for docsdocker ready!

Built with commit 3ef6e5a

https://deploy-preview-8357--docsdocker.netlify.com

@ghost ghost assigned paigehargrave Feb 28, 2019
@aki-k
Copy link

aki-k commented Mar 1, 2019

Please take this comment into consideration (about --ctdir option):

moby/moby#22054 (comment)

@paigehargrave
Copy link
Contributor Author

@aki-k - Thank you! I've added an additional blurb. PTAL - I'm not familiar with the subject matter, so I appreciate your guidance so that we can provide what is needed in the docs.

@bermudezmt
Copy link
Contributor

Hi @trapier and @stshow - can you PTAL at this PR? Thank you so much!

@bermudezmt bermudezmt added area/networking Relates to anything around networking OP/pending tech review labels Mar 12, 2019
@aki-k
Copy link

aki-k commented Mar 28, 2019

I needed to add the following to enable ping from the docker host to other internet hosts:

-A FILTERS -p icmp --icmp-type any -i eth0 -m conntrack --ctstate RELATED -j ACCEPT

Edit: I think this is the case if you set the INPUT policy to DROP.

@aki-k
Copy link

aki-k commented May 7, 2019

@paigehargrave Could you comment whether this is the method for securing the docker hosts' networking in the future or is there some other method that you are investigating?

Edit: I have no idea how this happened "aki-k unassigned shampabh 38 seconds ago".

@bermudezmt
Copy link
Contributor

@arkodg Is this something you can help us review and merge? There is an open comment on the original issue that needs to be incorporated: #8087 (comment)

@bermudezmt bermudezmt requested review from arkodg and removed request for trapier and stshow May 31, 2019 17:43
@arkodg
Copy link
Contributor

arkodg commented May 31, 2019

@bermudezmt I'd prefer documenting the rules in #8087 (comment)
as opposed to ones in the PR as an example of how users can add rules to the DOCKER-USER chain to make sure their rules take precedence over DOCKER rules

@aki-k
Copy link

aki-k commented Jun 4, 2019

@arkodg The iptables config in #8087 (comment) is not filtering e.g. Docker host machine's ssh port access, probably because it leaves the INPUT chain policy to ACCEPT and there's no -j in INPUT chain.

@arkodg
Copy link
Contributor

arkodg commented Jun 4, 2019

@aki-k this documentation is intended to help users limit and shape traffic entering the docker containers and so the DOCKER-USER chain can be used to apply rules before DOCKER's rules are applied to traffic directed to and from containers and #8087 (comment) shows some examples

If there are specific iptables rules and chains for traffic entering the host, I don't think they should be mentioned here (iptables rules are complex and users using them should know what they are doing)
https://github.com/boTux-fr/systemd-service-iptables/tree/master/etc/iptables is an example of how rules are split up

@aki-k
Copy link

aki-k commented Jun 5, 2019

iptables rules are not hard. What Docker did by bypassing the established iptables rules on a Docker host was the thing that created this problem. My goal is to have a simple setup which solves this problem.

The solution at https://unrouted.io/2017/08/15/docker-firewall/ is simple. It has one iptables rules file and one systemd service file. Using that setup can protect both the Docker host and the Docker containers but obviously needs testing to make sure both non-swarm and swarm setups work.

Currently you don't provide this kind of documentation and users are left to invent their own solutions.

@thaJeztah
Copy link
Member

thaJeztah commented Jun 6, 2019

Obligatory gif

56d5711699a4f3184b854dfecb740064

@aki-k
Copy link

aki-k commented Jun 6, 2019

@thaJeztah Are you saying that it's sheer luck that the iptables magic in the iptables chains is working and the wizard who did it left the company? :)

@thaJeztah
Copy link
Member

Haha, no, I posted the gif, because iptables is known to be hard, and it's very easy to shoot oneself in the foot, so many times when iptables is involved, that gif pops up.

@arkodg is definitely more familiar with this matter, and (for reasons above) having a good example here is super useful and welcome!

network/iptables.md Outdated Show resolved Hide resolved
@aki-k
Copy link

aki-k commented Jun 11, 2019

I may have understood wrong, but are you keeping the guidance on how to protect the Docker host's network with INPUT, FILTERS and FILTERS-LAN configuration?

https://github.com/docker/docker.github.io/blob/5857fa55d4a4e8930e1d75960cdf65ccbebdf32d/network/iptables.md

@paigehargrave
Copy link
Contributor Author

@arkodg You are the SME for this :) - please add whatever you think is appropriate. I removed the information that I thought you indicated was duplicate. If that was in error, please add it back in. This one needs to get wrapped up, but I want to make sure that whatever is needed is included. Thanks.

@aki-k
Copy link

aki-k commented Jun 17, 2019

@paigehargrave I meant that this commit

https://github.com/docker/docker.github.io/blob/5857fa55d4a4e8930e1d75960cdf65ccbebdf32d/network/iptables.md

still includes the configuration for INPUT, FILTERS and FILTERS-LAN that are meant for the Docker host's IP filtering, not for Docker containers' IP filtering. According to @arkodg this change should only concentrate on the DOCKER-USER iptables chain.

@aki-k
Copy link

aki-k commented Sep 13, 2019

@DawnWood-Docker

Do you know the status of this issue? There's been no comments since June 17.

Best regards,

Aki

@Dawn-Wood
Copy link
Contributor

Hi @aki-k Thank you for the bump. I tagged the engineer to close on one final comment, then will request a final review. Once that is done I will merge.

network/iptables.md Outdated Show resolved Hide resolved
network/iptables.md Outdated Show resolved Hide resolved
network/iptables.md Outdated Show resolved Hide resolved
Dawn-Wood and others added 6 commits September 13, 2019 08:58
Co-Authored-By: Arko Dasgupta <arkodg@users.noreply.github.com>
Co-Authored-By: Arko Dasgupta <arkodg@users.noreply.github.com>
Co-Authored-By: Arko Dasgupta <arkodg@users.noreply.github.com>
Co-Authored-By: Arko Dasgupta <arkodg@users.noreply.github.com>
Co-Authored-By: Arko Dasgupta <arkodg@users.noreply.github.com>
Copy link
Contributor

@Dawn-Wood Dawn-Wood left a comment

Choose a reason for hiding this comment

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

Hi @aki-k @arkodg and I reviewed and updated the content for technical accuracy. Thanks again for bringing this up. Will follow up with reviewers to get this merged.

@Dawn-Wood
Copy link
Contributor

@thaJeztah PTAL.

@Dawn-Wood
Copy link
Contributor

@thaJeztah I think we've resolved all of your comments but feel free to reopen any comments where that is not the case.

Copy link
Contributor

@Dawn-Wood Dawn-Wood left a comment

Choose a reason for hiding this comment

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

Hi @thaJeztah ,
I'm pretty sure we addressed your comments when I updated the pr with Arko. Happy to look at this again if that is not the case.

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.

whoops, sorry, this one dropped of my radar; I found some remaining (markdown related) issues

@@ -15,11 +15,15 @@ manipulate this table manually. If you need to add rules which load before
Docker's rules, add them to the `DOCKER-USER` chain. These rules are loaded
before any rules Docker creates automatically.

### Add a DOCKER-USER filter chain to allow persistent rules
This can be useful if you need to pre-populate `iptables` rules that need to be in place before
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a blank line between the header and the body? I seem to recall we've had cases where omitting the blank line caused some issues (and it's more consistent with the rest of the document)

Suggested change
This can be useful if you need to pre-populate `iptables` rules that need to be in place before
This can be useful if you need to pre-populate `iptables` rules that need to be in place before

@@ -49,6 +53,50 @@ the source and destination. For instance, if the Docker daemon listens on both
topic. See the [Netfilter.org HOWTO](https://www.netfilter.org/documentation/HOWTO/NAT-HOWTO.html)
for a lot more information.

### Filtering container traffic
The following example provides a set of filters and uses those filters for container and host traffic:
Copy link
Member

Choose a reason for hiding this comment

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

Same here

Suggested change
The following example provides a set of filters and uses those filters for container and host traffic:
The following example provides a set of filters and uses those filters for container and host traffic:


COMMIT
```
> **Note**: `--ctorigdstport` matches the destination port on the packet that initiated the connection,
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a blank line between the code-fence, and the note here as well?

Suggested change
> **Note**: `--ctorigdstport` matches the destination port on the packet that initiated the connection,
> **Note**: `--ctorigdstport` matches the destination port on the packet that initiated the connection,

```
> **Note**: `--ctorigdstport` matches the destination port on the packet that initiated the connection,
not the destination port on the packet being filtered. Therefore, responses to requests from Docker
to other servers have `SPT=80`, and match `--ctorigdstport 80`.
Copy link
Member

Choose a reason for hiding this comment

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

I think for notes, we repeat the > on each line throughout the docs, so;

> **Note**: `--ctorigdstport` matches the destination port on the packet that initiated the connection, 
> not the destination port on the packet being filtered. Therefore, responses to requests from Docker 
> to other servers have `SPT=80`, and match `--ctorigdstport 80`.


```bash
$ iptables-restore -n /etc/iptables.conf
```
Copy link
Member

Choose a reason for hiding this comment

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

This block now uses GitHub-flavor "fences", so should be un-indented, otherwise it shows up as a literal code block;

Screenshot 2019-10-07 at 13 49 38

@@ -58,4 +106,5 @@ for most users, because the `iptables` policies then need to be managed by hand.

## Next steps

- Read [Docker Reference Architecture: Designing Scalable, Portable Docker Container Networks](https://success.docker.com/Architecture/Docker_Reference_Architecture%3A_Designing_Scalable%2C_Portable_Docker_Container_Networks)
- Read [Docker Reference Architecture: Designing Scalable, Portable Docker Container Networks]
(https://success.docker.com/Architecture/Docker_Reference_Architecture%3A_Designing_Scalable%2C_Portable_Docker_Container_Networks)
Copy link
Member

Choose a reason for hiding this comment

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

looks like the added whitespace broke the link

To wrap links, the wrapping needs to be done in the links' "caption";

- Read [Docker Reference Architecture: Designing Scalable, Portable Docker
  Container Networks](https://success.docker.com/Architecture/Docker_Reference_Architecture%3A_Designing_Scalable%2C_Portable_Docker_Container_Networks)

That's a bit awkward sometimes, so an alternative could be to rephrase it slightly;

- Read "Docker Reference Architecture: Designing Scalable, Portable Docker
  Container Networks" on [success.docker.com](https://success.docker.com/Architecture/Docker_Reference_Architecture%3A_Designing_Scalable%2C_Portable_Docker_Container_Networks)

Or just don't wrap it (which I think in this case would be ok as an exception)

@aki-k
Copy link

aki-k commented Jan 27, 2020

@DawnWood-Docker

Hi,

Did this documentation update drop off the radar? I thought there was drive to get it included.

Best regards,

Aki

@docker-robott
Copy link
Collaborator

Thanks for the pull request. We'd like to make our product docs better, but haven’t been able to review all the suggestions.
As our docs have also diverged, we do not have the bandwidth to review and rebase old pull requests.

If the updates are still relevant, review our contribution guidelines and rebase your pull request against the latest version of the docs, then mark it as fresh with a /remove-lifecycle stale comment.
If not, this pull request will be closed in 30 days. This helps our maintainers focus on the active pull requests.

Prevent pull requests from auto-closing with a /lifecycle frozen comment.

/lifecycle stale

@crazy-max crazy-max deleted the static-ip-tables-rules branch January 14, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/networking Relates to anything around networking lifecycle/stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants