voxpupuli / puppet-nftables

Puppet Module to manage nftables firewall rules.
Apache License 2.0
12 stars 33 forks source link

Regex to validate chain names #174

Open passowrd opened 1 year ago

passowrd commented 1 year ago

Hi

I'm trying to manage our current firewall rules with puppet. We currently use chains that contain a dash in their name. It seems that this modules does not allow dashes in chain names and in RuleName.

https://github.com/voxpupuli/puppet-nftables/blob/master/manifests/chain.pp#L5 https://github.com/voxpupuli/puppet-nftables/blob/master/types/rulename.pp#L6

I'm not sure as to why though. It is definitely not a limitation of nftables (I encountered the problem while trying to import existing chains named with a dash). It seem that the dash is used as a separator in the module to distinguish chains and rules name. But this disallow valid chains names.

Do you think it would it be possible to use a different char to split chain names and rules.

Also while testing, I noticed that the dot, which is also a valid nft table name char (even as first char), is also not included in the allowed regex.

Affected Puppet, Ruby, OS and module versions/distributions

How to reproduce (e.g Puppet code you use)

nftables::rule{'default_in-CUSTOM-IN':
    content => 'ip protocol tcp jump CUSTOM-IN',
}

What are you seeing

Error: Evaluation Error: Error while evaluating a Resource Statement, Nftables::Rule[default_in-CUSTOM-IN]: parameter 'rulename' expects a match for Nftables::RuleName = Pattern[/^[a-zA-Z0-9_]+-[a-zA-Z0-9_]+(-\d+)?$/], got 'default_in-CUSTOM-IN' (file: /mnt/h2_firewall/manifests/init.pp, line: 99) on node puppetdev.lxd

What behaviour did you expect instead

no error with valid nftables chain names ;)

Output log

Any additional information you'd like to impart

duritong commented 1 year ago

It's because we need a seperator that is valid for a puppet resource title AND that we can use to split it so it can be passed down to the nftables configuration: https://github.com/voxpupuli/puppet-nftables/blob/master/manifests/config.pp#L32-L39

This is quite at the core of this module.

Happy to look at backward compatible patches.