[BUG] nftables state module not idempotent for conntrack

[lkubb]

Description nftables.append keeps appending rules that have connstate set.

Edit: This also applies for if/of with a different cause (nft list does not include meta before iifname/oifname).

Edit2: This also applies for jump: dnat with to-destination set (nft list prints dnat to <target>, not dnat <target>).

Setup Salt minion on Debian 11 VM

Steps to Reproduce the behavior

testtable:
  nftables.table_present: []

testchain:
  nftables.chain_present:
    - table: testtable

This is not idempotent:
  nftables.append:
    - table: testtable
    - chain: testchain
    - jump: accept
    - connstate: established,related

Expected behavior Be idempotent.

Screenshots

$ nft list ruleset
$ salt-call state.apply nft_idem
$ nft list ruleset
table ip testtable {
	chain testchain {
		ct state { established, related } accept
	}
}
$ salt-call state.apply nft_idem
$ nft list ruleset
table ip testtable {
	chain testchain {
		ct state { established, related } accept
		ct state { established, related } accept
	}
}

Versions Report

salt --versions-report

(Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)

Salt Version:
          Salt: 3004.2

Dependency Versions:
          cffi: Not Installed
      cherrypy: Not Installed
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.11.3
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.0
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: Not Installed
      pycrypto: Not Installed
  pycryptodome: 3.9.7
        pygit2: Not Installed
        Python: 3.9.2 (default, Feb 28 2021, 17:03:44)
  python-gnupg: Not Installed
        PyYAML: 5.3.1
         PyZMQ: 20.0.0
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.4

System Versions:
          dist: debian 11 bullseye
        locale: utf-8
       machine: x86_64
       release: 5.10.0-13-amd64
        system: Linux
       version: Debian GNU/Linux 11 bullseye

Additional context This behavior is caused by nftables.check execution module not recognizing the rule correctly. It lists the rules with the --numeric switch, which translates the connection states to hex values, but searches for them using their string representation.

Example output:

$ nft --handle --numeric --numeric --numeric list chain ip testtable testchain # why 3x numeric btw? copied from source
table ip testtable {
	chain testchain { # handle 1
		ct state { 0x2, 0x4 } accept # handle 3
		ct state { 0x2, 0x4 } accept # handle 5
	}
}

Workaround possible by specifying the values in hex (which should break the analogy to iptables afaik):

testtable:
  nftables.table_present: []

testchain:
  nftables.chain_present:
    - table: testtable

This is idempotent:
  nftables.append:
    - table: testtable
    - chain: testchain
    - jump: accept
    - connstate: '0x2, 0x4'

[mlalpho]

Thank you for reporting. I noticed the hex weirdness in addition to https://github.com/saltstack/salt/pull/61975. following 👀

[redbaron4]

I ran into this today.

This happens because modules/nftables.py in check() uses --numeric argument to nft which converts many things (including connstate) to numeric.

However, removing the --numeric arguments to nft command does not solve this issue because when salt builds the command it encapsulates connstate and port in {}, e.g. { new } or {established,related}, but when salt runs nft to print the rules (so that it can search for exact match), the nftables removes the {} for single connstate or port.

Consider a state element

allow_dns_udp:
  nftables.append:
    - save: True
    - table: filter
    - chain: OUTPUT
    - jump: accept
    - match: state
    - connstate: new
    - proto: udp
    - dport: 53
    - destination: 10.1.1.1

Thus if salt built the following rule

ct state { new } ip daddr 10.1.1.1 udp dport { 53 } accept

when it echoes the existing rules (without --numeric), it gets

ct state new ip daddr 10.1.1.1 udp dport 53 accept

Hence it can never match the rule exactly. The rule matching logic needs to probably use regular expressions for this issue to be fixed.