garethr / garethr-docker

Puppet module for managing docker
Apache License 2.0
397 stars 531 forks source link

Need $docker::manage_firewall option #129

Open hesco opened 10 years ago

hesco commented 10 years ago

Hey Gareth:

It seems that while docker is managing the creation of rules in the FORWARD chain, that it leaves the policies on every chain set to ACCEPT and the docker host particularly vulnerable, with all ports wide open. I want to use the puppetlabs/firewall module to lock things down and to manage the ports I need to open to the world for my haproxy, db servers and other services. But I want to do that without messing up what is happening with the docker rule set.

This evening I had a short exchange on the #docker channel on this subject. I'm imagining a new defined type docker::firewall which would be invoked by docker::run, and perhaps as well by a new docker::firewall::legacy_containers (might not be the right name there) which would handle managing the firewall for already running containers or containers which were run directly with the docker client, and in a way not otherwise managed by puppet.

23:50:12 hesco | I'm nearly ready to bring the iptables rule set run on my docker hosts under puppet management with the puppetlabs-firewall module. But the first thing the firewall module will do is flush whatever is already there. My question is this? What will it take to make docker rebuild its own rule set on top of what I'm doing with puppet? And how long would that take? Can't really afford to break things at this hour, when I'm hoping to go to bed soon. Can anyone advise me on how docker manages its iptables rule set and what might be expected if I run this test?

00:03:20 akerl | hesco: In my experience, if you're doing other things with iptables, it quickly becomes saner to have $other_thing manage all the rules and tell Docker to leave iptables alone

00:07:35 hesco | akerl: ok, I'll buy that. So it sounds like I need some sort of new defined type which will harvest the published ports out of docker ps and convert those into rules for the FORWARD chain, perhaps.

00:08:06 akerl | That would be snazzy

00:10:48 hesco | sounds like a patch for the garethr/docker module.

So, is this something which seems germane to your module, as I imagine? Would you consider such a patch? Or should I develop it under a distinct namespace?

-- Hugh

hesco commented 9 years ago

UPDATE:

I have implemented this as weave::firewall::docker on the hesco/weave project, but feel that code really belongs here on the garethr/docker project instead. See:

https://github.com/hesco/hesco-weave/blob/master/manifests/firewall/docker.pp https://github.com/hesco/hesco-weave/blob/master/manifests/firewall/dnat_published_port.pp

At version v0.7.45 of the weave module I have been working on, this code has provided me with a stable cross-docker-host weave network for the past two days now where it is being used to host production services. I am close to tagging a new release for the forge to make this code more publicly available.

My question to you is this: Would you be open to merging a pull request which moves this functionality out of the weave module and into the docker module where it really belongs? I would continue to maintain the weave::firewall::weave class and the weave::firewall::listen_to_peer type on the hesco/weave project.

-- Hugh

garethr commented 9 years ago

Hi @hesco, I'd certainly be open to including this in the docker module, assuming it's something that people explicitly include.

I think the trick will be testing it with the supported operating systems, although I'm happy for that to start with partial support and warnings if that's easier.

hesco commented 9 years ago

What I intend is a straight port of the existing weave::firewall::docker manifest. The ::firewall::docker class relies entirely on six nearly static calls to puppetlabs/firewall's firewall defined type and one of those built with the $::network_docker0 fact. That fact is the only thing the least dynamic about the class. It is otherwsie static code. It has been running in my production environment now for that past several days and you are welcome to give it a code review at:

https://github.com/hesco/hesco-weave/blob/master/manifests/firewall/docker.pp

The other piece which belongs in the garethr/docker module is a defined type. It adds two rules using two of three invocations of the same firewall defined type, routed by a conditional. You can review it here:

https://github.com/hesco/hesco-weave/blob/master/manifests/firewall/dnat_published_port.pp

I'm still swamped by a few other tasks before I can get to this, but if you want to be patient about it, I will package the port up with some documentation on its use, some rspec tests and send you a PR. It will all be wrapped up in a $docker::manage_firewall hiera setting. After you merge, I will remove that code from the weave project and simply call it in your namespace instead.

Ahead of it in line though it a PR adding debian support to gareth/docker. Already been working on that in a spare console while doing other things.

-- Hugh

On Sun, 16 Nov 2014 10:52:14 -0800 Gareth Rushgrove notifications@github.com wrote:

Hi @hesco, I'd certainly be open to including this in the docker module, assuming it's something that people explicitly include.

I think the trick will be testing it with the supported operating systems, although I'm happy for that to start with partial support and warnings if that's easier.


Reply to this email directly or view it on GitHub: https://github.com/garethr/garethr-docker/issues/129#issuecomment-63232584

Hugh Esco hesco@yourmessagedelivered.com

diranged commented 9 years ago

This looks like a great addition, and is definitely something we're running into as a blocking issue. In the short term, since our hosts are behind a VPC, we can probably just open the firewall up ... but in the long run, we definitely want docker::run to manage firewall settings for every single container.

That said, I agree that it generally needs to be able to handle all containers that are running, even if they aren't run through puppet. We're experimenting with ECS now and obviously that means that Puppet won't be involved in most of the docker container execution.

arioch commented 9 years ago

@hesco: I've manually added your firewall patch to my test setup and it seems to be triggering a bit of a chicken&egg problem.

Applying the firewall rules using Puppet works flawlessly. However when restarting the Docker service it complains that it can't insert a forward rule because it already exists.

When removing those firewall rules from the manifest they are purged from the Docker host as well. After restarting the Docker service all is well again.

Any thoughts about how to get around this problem?

hesco commented 9 years ago

Hey Tom:

Thanks for pointing this out. I too have been seeing this in my cluster as well.

Every docker host restart, or even a docker daemon restart requires first a flush of iptables. I have a bash script at /root/lib/sh/flush_iptables.sh script (installed by my puppet manifests) to handle that for me, but have been handling that step manually myself to date.

My plan has been to port the weave::firewall::docker over to the garethr module as garethr::docker::firewall. Gareth has already expressed interest in accepting such a patch, but I have not gotten around to getting that assembled for him yet.

When I do that, it would make sense to add perhaps: garethr::docker::firewall::enable garethr::docker::firewall::flush

and create a dependency on the latter for the Service['docker.io'] resource.

Feel free to create a bug report on the hesco-weave project to that effect, if I do not get to it before you do (and given what is on my plate this week, that would not be hard to imagine).

Thanks again for pointing this out. Although I had seen this same behavior, I had not yet focused on it as a bug in the puppet module demanding my attention. But that makes perfect sense to get this automated.

-- Hugh

On Wed, 07 Jan 2015 01:47:46 -0800 Tom De Vylder notifications@github.com wrote:

@hesco: I've manually added your firewall patch to my test setup and it seems to be triggering a bit of a chicken&egg problem.

Applying the firewall rules using Puppet works flawlessly. However when restarting the Docker service it complains that it can't insert a forward rule because it already exists.

When removing those firewall rules from the manifest they are purged from the Docker host as well. After restarting the Docker service all is well again.

Any thoughts about how to get around this problem?


Reply to this email directly or view it on GitHub: https://github.com/garethr/garethr-docker/issues/129#issuecomment-68999515

Hugh Esco hesco@yourmessagedelivered.com

diranged commented 9 years ago

Any update on merging the firewall code into this puppet module?

hesco commented 9 years ago

@diranged: Nothing yet. Completed one project last night, starting another today. This and other work on the hesco-weave project remain on my to-do list. In the mean time, you can install hesco-weave and use the weave::firewall::docker and weave::firewall::dnat_published_port defined types it makes available. If your docker installation spans multiple physical hosts, you might also find useful the weave SDN router functionality in that module. But it is not necessary to implement weave to use the docker specific defined types which hesco-weave (https://forge.puppetlabs.com/hesco/weave | https://github.com/hesco/hesco-weave) exposes. My own production services have been offered on a stable docker cluster running on this code now since mid-November.

zveljkovic commented 9 years ago

Please see this link for solution if using puppet labs firewall: https://tickets.puppetlabs.com/browse/MODULES-1234

and from there this link: https://github.com/puppetlabs/puppetlabs-firewall#parameters-1

Solution for this is to remove if you have it anywhere resources { "firewall": purge => true } and to put below to prevent purging docker rules in your pre firewall

Ignore docker dynamic rules

firewallchain { 'DOCKER:nat:IPv4': purge => true, ignore => ['172.17.'], }-> firewallchain { 'POSTROUTING:nat:IPv4': purge => true, ignore => ['172.17.'], }-> firewallchain { 'DOCKER:filter:IPv4': purge => true, ignore => ['172.17.'], }->

inside my_firewall::pre ////"Firewall { before => Class['my_firewall::post'], require => Class['my_firewall::pre'] }" for example below

class my_firewall::pre { Firewall { require => undef, }

Ignore docker dynamic rules

firewallchain { 'DOCKER:nat:IPv4': purge => true, ignore => ['172.17.'], }-> firewallchain { 'POSTROUTING:nat:IPv4': purge => true, ignore => ['172.17.'], }-> firewallchain { 'DOCKER:filter:IPv4': purge => true, ignore => ['172.17.'], }->

....

salimane commented 9 years ago

any update on this ? Thanks

hesco commented 9 years ago

@salimane:

check out: https://github.com/hesco/hesco-weave/blob/master/manifests/firewall/docker.pp https://github.com/hesco/hesco-weave/blob/master/manifests/firewall/dnat_published_port.pp

Afraid a new job has further delayed my recrafting this work as a PR for the garethr-docker module, but with an: include weave::firewall::docker and the addition of appropriate invocations of the weave::firewall::dnat_published_port defined type for each port EXPOSE'd by a Dockerfile, or opened using the -p switch to docker run, you can use the code where it already exists, without waiting on me to prepare that PR. But note the caveat described above by @arioch, as well as my response to it.

Good luck and feel free to ping me again if you have any trouble implementing that.

-- Hugh

JamieCressey commented 9 years ago

+1 to this. We use puppetlabs-firewall so the docker defaults are overwritten with a subsequent puppet run.

hesco commented 9 years ago

@JamieCressey :

for the short answer, see my comment above: https://github.com/garethr/garethr-docker/issues/129#issuecomment-114843540