Redesign of the firewall rule configuration

[drybjed]

Oh yeah, I still need to update the documentation for the new ferm rules.

[drybjed]

@gaudenz, you might want to check this PR out. The Travis test doesn't pass due to the missing documentation (I'm working on it), but the role should work fine. A review would be nice. :-)

[drybjed]

@ypid Apart from outdated documentation, the changes to the firewall configuration should be completed now. Would you like to test them on some hosts and report back any issues?

[ypid]

Would you like to test them on some hosts and report back any issues?

I am currently working on other things that includes building myself a staging/test environment for DebOps similar as you already have. I will check back when that is done and then maybe also have updated docs for the role :wink:

I admire your work, as always :+1:

[Gomez]

This redesign solved my forwarding issue. Unfortunately i cant reproduce what was wrong with current master/ferm config. This PR works fine with all my ferm inv configs (accept template, dmz forwarding). It also works with existing roles, like debops.samba.

[drybjed]

@ypid I modified the rule parser again and dropped the list/dictionary merge, only lists are supported now. I think that the role is ready for a release, apart from the changes you mentioned; I'll look at them now.

I'm still not really satisfied about what the role has become, but I suppose it can't be helped if backwards compatibility needs to be preserved. This is what happens when you try to design a language to generate another language in a third language, I guess. Perhaps when nftables support is brought to DebOps this will be designed better. I think that for now this is fine, time to wrap this PR up and start working on other things.

[drybjed]

I suppose one of the reasons would be to have a consistent data definition model. Allowing for both YAML dictionaries and YAML lists at the same time quickly shown an issue with dependent variables from multiple roles - they could only be defined as a list, and because of that came a restriction on using YAML dictionaries via dependent variables. Switching to only YAML lists as input made things clearer.

Internally not much has changed. The role still parses the lists and combines them do a YAML dictionary which is then used by the configuration template, just using a python .copy() method instead of a crazy redundant Jinja macro. I hoped that this will make the role a bit faster, but it looks that variable computation is not a culprit here, but generation of the configuration files themselves.

One advantage the new internal mechanism has over the old implementation is that it could be used to generate all of the ferm configuration in one file with a suitable template. Adding support for storing the dependent configuration in the secret/ directory would even allow to track configuration defined by other roles. The entries can be sorted by their weight and type values. Perhaps this would make the role a bit faster. But I would rather explore that at some other time, or perhaps with nftables instead, in the future.

[ypid]

Thanks. Sounds good to me. I will review later when other roles like sshd are updated and can be tested with this as well. Already the case, missed that.

Adding support for storing the dependent configuration in the secret/ directory would even allow to track configuration defined by other roles.

My idea would be to only do this when it is required because applications can not handle conf.d style multiple files. I don’t see the advantage to also do this for applications which are state-of-the-art to support conf.d style. Am I missing something? Remote hosts are already "track configuration" for us.

nftables

Not sure if we need to care much about it. Basically "a language to generate another language in a third language". One of those intermediate languages should take care of this for us, else what is the point of all this ;-)

[drybjed]

Actually, the pending PRs to debops.sshd, debops.ifupdown and debops.tinc rely on the features of this PR to work correctly, ie. generate the expected firewall rules. You could add these PRs to your development environment and test them that way, but to keep the roles working, the debops.ferm PR needs to be merged first.

About putting all in one firewall config script - currently the default ferm rule files are generated in excess of 25 seconds on my testing systems. Because debops.ferm can be executed multiple times in a playbook, this time increases linearly, therefore I would try and look into ways to make this faster. Generating a configuration file one time should be a bit faster than generating a bunch of small files.

I haven't yet looked into comparsion between iptables and nftables, but since the latter supports a ferm-like configuration files "natively", it should be easier to handle, rather than generating a bunch of iptables rules using Perl (ferm). I'm not sure about support by other applications though, like libvirt. I guess that we still have some time.

[drybjed]

To add one more reason for a switch from YAML dictionaries to YAML lists, is that the former do not allow for conditional activation of pararell sets of configuration, while the latter allows this. For example, you can't have different version of the same property selected conditionally in a dictionary:

ferm__rules:

  'accept-http':
    dport: [ '80', '443' ]
    state: '{{ "present" if nginx_pki|bool else "absent" }}'

  'accept-http':
    dport: [ '80' ]
    state: '{{ "absent" if nginx_pki|bool else "present" }}'

In this case Ansible would select only the first "entry" in the dictionary and use that. However, with a YAML list you can have multiple entries that define the same property and parse them in order to select the specific one:

ferm__rules:

  - name: 'accept-http'
    dport: [ '80', '443' ]
    state: '{{ "present" if nginx_pki|bool else "absent" }}'

  - name: 'accept-http'
    dport: [ '80' ]
    state: '{{ "absent" if nginx_pki|bool else "present" }}'

[drybjed]

@ypid, just a heads up, this PR is blocking the merge for other PRs in debops.sshd, debops.ifupdown and debops.tinc. Since there were some changes recently, I'm still waiting for your approval. Any chance to have it in the next week or so? :-)

[ypid]

Will try :)

[drybjed]

@ypid I'll do that in the morning. In the meantime, I'm looking into writing filter plugins, so code like this will be moved out from templates and into Python.

[drybjed]

@ypid Latest PR merged, waiting for green light.

[drybjed]

Great, thanks, I'll merge this and bunch of other PRs tommorow. Let the grant breaking begin.

I actually thought for a moment if a switch to filter-based approach would have a merit at this point, but I guess the current solution is good enough. There are other roles that need to be updated first before I get back to debops.ferm again.