firewall module to accept array of sources/dests

[Alex Harvey]

Hi all,

I just ran into an issue with the firewall module

https://tickets.puppetlabs.com/browse/MODULES-3066

It looks like I'm not the first user of this module to be surprised to discover that it can't do this:

firewall { '100 allow http and https access':

source => [

'1.1.1.1/24',

'1.1.2.1/24',

],

dport => [80, 443],

proto => tcp,

action => accept,

}

Apparently there is a long history of people having a go at adding this feature and then giving up.

Well, I am that sort of person who doesn't feel comfortable in a world where the above declaration is impossible, therefore will volunteer to add the feature.

Can I get some feedback at this early stage that my PR would be accepted, assuming I can come up with a clean, working solution?

Thanks,

Alex

-- 
Alex Harvey
RAZOR Consulting

http://razorconsulting.com.au
T: +61 409 665 227

[Felix Frank]

On 02/09/2016 06:41 AM, Alex Harvey wrote:
> Can I get some feedback at this early stage that my PR would be
> accepted, assuming I can come up with a clean, working solution?

Hi,

I don't think that anyone will be able to answer this without at least
looking at what you're building, or intend to.

From experience, cool features like this have good chances, *unless*
they come with some pitfalls or a catch that the maintainer (Puppet
Labs?) is not willing to accept.

As for the feature you're looking at: My gut tells me that you might not
be able to come up with a clean model to support all that. Multiple
destination ports should not be problematic, thanks to netfilter's
multiport module.

But multiple addresses get expanded into distinct rules, IIRC. This
likely cannot be reconciled with Puppet's resource model, or not without
introducing some bizarre semantic tricks.

So my advice is to open a PR as soon as possible, even if the feature
does not work yet, just to showcase your approach and gather the
feedback you came seeking here.

HTH,
Felix

[Hunter Haugen]

The difficulty with allowing multiple sources is that it falls in line only with a scripted workflow, not an idempotent workflow. This is from the iptables manpage: "Multiple addresses can be specified, but this will expand to multiple rules (when adding with -A), or will cause multiple rules to be deleted (with -D)."

Converting the firewall type to manage multiple rules with a single resource is surely the way to madness ;).

-Hunter

--
You received this message because you are subscribed to the Google Groups "Puppet Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-users...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/56BD0C52.80307%40Alumni.TU-Berlin.de.

For more options, visit https://groups.google.com/d/optout.

[Alex Harvey]

Hi Felix, Hunter

Thanks for the responses.

There's no doubt that it's a tricky feature to write, but having the feature would be a big win for the community and my customers.  So I'd really like to do this, and I'm sure if we put our heads together we can come up with the best design possible.

I am playing in my mind with a few ways of doing this.

1.

Suppose we have two rules as follows:

ACCEPT     tcp  --  1.1.1.1/24           0.0.0.0/0           multiport dports 80,443 /* 100 allow http and https access */ 

ACCEPT     tcp  --  2.2.2.2/24           0.0.0.0/0           multiport dports 80,443 /* 100 allow http and https access */ 

The provider could be modified so as to represent these as:

firewall { '100 allow http and https access':                                                                                                                                  

  ensure             => 'present',

  action             => 'accept',

  chain              => 'INPUT',

  checksum_fill      => 'false',

  clamp_mss_to_pmtu  => 'false',

  clusterip_new      => 'false',

  dport              => ['80', '443'],

  isfragment         => 'false',

  kernel_timezone    => 'false',

  log_uid            => 'false',

  physdev_is_bridged => 'false',

  proto              => 'tcp',

  random             => 'false',

  rdest              => 'false',

  reap               => 'false',

  rsource            => 'false',

  rttl               => 'false',

  socket             => 'false',

  source             => ['1.1.1.1/24', '2.2.2.2/24'],

  table              => 'filter',

  time_contiguous    => 'false',

}

How?  I don't know yet.  At first glance it's hard to see why this would be a hugely difficult coding problem, but I suspect that many unpleasant edge-cases that I haven't thought of yet lie in wait for me, and that's why I lean to option 2.

2.

Add to the firewall module (or perhaps a new Forge module) a defined type that wraps around the existing firewall types/providers.  In Puppet 4, that should be easy to do in the DSL using an iterator; but because I'd like to support Puppet 3 as well, it's a bit trickier.  Still, quite doable.  The hardest part seems to be thinking up a name for the new type.  Any suggestions?

Best regards,

Alex

[Felix Frank]

On 02/12/2016 07:11 AM, Alex Harvey wrote:
>
> ACCEPT tcp -- 1.1.1.1/24 0.0.0.0/0 multiport dports
> 80,443 /* 100 allow http and https access */
>
> ACCEPT tcp -- 2.2.2.2/24 0.0.0.0/0 multiport dports
> 80,443 /* 100 allow http and https access */
>
>
> The provider could be modified so as to represent these as:

<snip>

Conceptionally, it might just work. But it would be quite hard, and
create a maintenance nightmare. (Have you *looked* at the current
provider instances/parsing methods? Oh my...)

> 2.
>
> Add to the firewall module (or perhaps a new Forge module) a defined
> type that wraps around the existing firewall types/providers. In
> Puppet 4, that should be easy to do in the DSL using an iterator; but
> because I'd like to support Puppet 3 as well, it's a bit trickier.
> Still, quite doable. The hardest part seems to be thinking up a name
> for the new type. Any suggestions?

While naming things *is* one of the hardest problems in software, I'm
sure we can figure something out on this one. No worries.

Iterating on the DSL level is nice and all, but it will cause issues for
users who don't purge unmanaged firewall rules (granted, that should be
a rare issue, but then I'm willing to bet that there are people with
weird edge cases like that.)

The problem is that removing sources from the array of your multiplexer
resource will just lead to some firewall resources not being in the
catalog anymore. Their respective rules will remain orphaned, which is
not what the user will expect.

Cheers,
Felix

[Alex Harvey]

On 13 February 2016 at 09:57, Felix Frank <Felix...@alumni.tu-berlin.de> wrote:

On 02/12/2016 07:11 AM, Alex Harvey wrote:

ACCEPT     tcp  --  1.1.1.1/24           0.0.0.0/0   multiport dports 80,443 /* 100 allow http and https access */
ACCEPT     tcp  --  2.2.2.2/24           0.0.0.0/0 multiport dports 80,443 /* 100 allow http and https access */

The provider could be modified so as to represent these as:

<snip>

Conceptionally, it might just work. But it would be quite hard, and create a maintenance nightmare. (Have you *looked* at the current provider instances/parsing methods? Oh my...)

Yeah, I have.  Main reason I'm leaning to (2). :)

 

2.

Add to the firewall module (or perhaps a new Forge module) a defined type that wraps around the existing firewall types/providers.  In Puppet 4, that should be easy to do in the DSL using an iterator; but because I'd like to support Puppet 3 as well, it's a bit trickier.  Still, quite doable.  The hardest part seems to be thinking up a name for the new type.  Any suggestions?

While naming things *is* one of the hardest problems in software, I'm sure we can figure something out on this one. No worries.

Iterating on the DSL level is nice and all, but it will cause issues for users who don't purge unmanaged firewall rules (granted, that should be a rare issue, but then I'm willing to bet that there are people with weird edge cases like that.)

The problem is that removing sources from the array of your multiplexer resource will just lead to some firewall resources not being in the catalog anymore. Their respective rules will remain orphaned, which is not what the user will expect.

Is this really a problem though?  The documentation for the module recommends that users do purge the unmanaged firewall rules.  If they choose not to, then they should understand that means they need to take care of those manually.  It's no different to any other resource in Puppet.  If one day I stop managing the /etc/motd file, I should understand that Puppet won't delete the file; it'll simply leave it in whatever state it was in.

[Felix Frank]

On 02/13/2016 05:34 AM, Alex Harvey wrote:

The problem is that removing sources from the array of your multiplexer resource will just lead to some firewall resources not being in the catalog anymore. Their respective rules will remain orphaned, which is not what the user will expect.

Is this really a problem though?  The documentation for the module recommends that users do purge the unmanaged firewall rules.  If they choose not to, then they should understand that means they need to take care of those manually.  It's no different to any other resource in Puppet.  If one day I stop managing the /etc/motd file, I should understand that Puppet won't delete the file; it'll simply leave it in whatever state it was in.

Sure, but I feel that this case is especially confusing.

The user does not remove a resource from their manifest. They change a parameter of one of their resources, which feels like changing a property value for a proper resource. The fact that this may not be sync'ed correctly by the agent can be surprising, and removing firewall rules is a highly critical operation.

So, yes, I think you should go ahead and build that module, but please make sure to plaster its documentation with warnings ;-)

Cheers,
Felix

[Alex Harvey]

On 14 February 2016 at 00:30, Felix Frank <Felix...@alumni.tu-berlin.de> wrote:

Sure, but I feel that this case is especially confusing.

The user does not remove a resource from their manifest. They change a parameter of one of their resources, which feels like changing a property value for a proper resource. The fact that this may not be sync'ed correctly by the agent can be surprising, and removing firewall rules is a highly critical operation.

So, yes, I think you should go ahead and build that module, but please make sure to plaster its documentation with warnings ;-)

OK, noted. 

I have decided that I will create a new Puppet Forge module for this, one for Puppet 3 and a separate one for Puppet 4.  This way I can avoid creating a new support burden for the team that manages Puppet Labs firewall and still deliver the features needed.  If it proves to be popular, I'll be happy to have it merged into the support Puppet Forge firewall module at any time.

It will deliver just a single defined type (and the Puppet 3 version will also deliver a private defined type to workaround the lack of iterator.). 

As far as the naming is concerned I wish I could call it:

firewall::multi 

That would be nice because it could be moved to the firewall module at a later date and no one using it would need to refactor.  However that would result in a module name clash with the Puppet Labs firewall module, which is a dependency.  

So I think I'll call it:

firewall_multi

It will basically accept any parameter that firewall accepts and pass it straight through to the firewall resource, unless that parameter is the source or destination, in which case it will of course loop through these arrays, spawing one firewall resource for each.

[Felix Frank]

On 02/19/2016 04:00 AM, Alex Harvey wrote:
> So I think I'll call it:
>
> firewall_multi
>
> It will basically accept any parameter that firewall accepts and pass
> it straight through to the firewall resource, unless that parameter is
> the source or destination, in which case it will of course loop
> through these arrays, spawing one firewall resource for each.

:thumbs_up: :-)

[Alex Harvey]

Hi all,

I have a working prototype and any feedback would be welcome.

https://github.com/alexharv074/puppet-firewall_multi

Kind regards,

Alex

--
You received this message because you are subscribed to a topic in the Google Groups "Puppet Users" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/puppet-users/6pF_AP3lZbk/unsubscribe.
To unsubscribe from this group and all its topics, send an email to puppet-users...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-users/56C8F68B.4000504%40Alumni.TU-Berlin.de.