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

Need $docker::manage_firewall option #129

Open
hesco opened this issue Oct 13, 2014 · 13 comments
Open

Need $docker::manage_firewall option #129

hesco opened this issue Oct 13, 2014 · 13 comments
Labels

Comments

@hesco
Copy link

hesco commented Oct 13, 2014

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
Copy link
Author

hesco commented Nov 16, 2014

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
Copy link
Owner

garethr commented Nov 16, 2014

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
Copy link
Author

hesco commented Nov 17, 2014

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 [email protected] 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:
#129 (comment)

Hugh Esco [email protected]

@diranged
Copy link

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
Copy link
Contributor

arioch commented Jan 7, 2015

@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
Copy link
Author

hesco commented Jan 7, 2015

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 [email protected] 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:
#129 (comment)

Hugh Esco [email protected]

@diranged
Copy link

diranged commented Feb 6, 2015

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

@hesco
Copy link
Author

hesco commented Feb 6, 2015

@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
Copy link

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
Copy link
Contributor

any update on this ?
Thanks

@hesco
Copy link
Author

hesco commented Jun 24, 2015

@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
Copy link

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

@hesco
Copy link
Author

hesco commented Jul 23, 2015

@JamieCressey :

for the short answer, see my comment above:
#129 (comment)

cegeka-jenkins pushed a commit to cegeka/puppet-docker that referenced this issue Apr 17, 2024
* removing obsolete code

* fixing acceptance tests

* incorporating updates from markw/puppetlabs-docker

* fixing params

* putting in systemd default for run on redhat

* removing old variable

* removing archilunx template

* updating last of the spec tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants