Salt: nftables state of limited use

[antifob]

Description of Issue/Question

nft has a specific syntax which the nftables state tries to handle somehow. First it tries to pick and choose from the syntax elements it knows about to create the command to execute and, second, it doesn't support adding additional elements. Effectively, this completely limits the user to what the nftables state supports.

Setup

(Please provide relevant configs and/or SLS files (Be sure to remove sensitive info).)

Steps to Reproduce Issue

Just try to execute the following command with salt:
nft add rule filter output ip daddr 1.1.1.1 tcp dport 443 accept

The nftables module doesn't not support the ip syntax element.

Versions Report

Salt 2018.03.03

[Ch3LL]

just to clarify. are you requesting that this specific command be added to the module?

[antifob]

Actually, no. It should be added, but the problem here, IMO, is that there are no mechanisms to add additional elements to the generated command line if salt doesn't support them.

I also noticed the iptables module has the same design issue.

[Ch3LL]

we could just add **kwargs to the call, but want to get @saltstack/team-core 's opinion here. Do we want to allow just any kwarg to be passed into this module?

[waynew]

@antifob Does salt minion nft.append filter output rule="output ip daddr 1.1.1.1 tcp dport 443 accept" not work for you? I'm not 100% sure that's the right syntax, but I think it is.

[antifob]

For the record, the correct syntax is nftables.append filter output rule="ip daddr 1.1.1.1 tcp dport 443 accept". And yes it works because the rule is passed straight down to the execution step.

I'm talking about the nftables state. I renamed and edited my report accordingly. Sorry for the confusion.

[waynew]

Hey @antifob thanks for the follow-up!

Do you have your sample ntftables state that fails? Looking at the code, it looks like:

add_nft_rules:
  nftables.append:
    - table: filter
    - chain: output
    - rule: ip daddr 1.1.1.1 tcp dport 443 accept

or something similar should work. What state are you using? (I didn't see it in your report, but I'm not above blaming GitHub/Chrome for caching something :trollface:)

[antifob]

In this particular case, nftables reports success when nothing was added to the ruleset. Here is the output:

          ID: add_nft_rules
    Function: nftables.append
      Result: True
     Comment: Set nftables rule for add_nft_rules to:  for ipv4
     Started: 00:43:22.103772
    Duration: 0.418 ms
     Changes:
              ----------
              locale:
                  add_nft_rules

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:   0.418 ms
root@localhost:/srv# nft list ruleset
table ip filter {
        chain output {
                type filter hook output priority 0; policy accept;
        }
}

I'm linking to #51641 on this one and the rule still isn't added.

[garethgreenaway]

I looked at this last night and see several issues with the state and module. Assigned it to myself and will be spending some time looking at it.

[waynew]

All yours, then, @garethgreenaway :)

[garethgreenaway]

@waynew I made the mess, only fitting that I clean it up :smile:

[elipsion]

Somewhat related, @garethgreenaway, could you also look at adding support for sets?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[waynew]

@elipsion does Gareth's PR fix your issue here?

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.