saltstack / saltstack/salt

[BUG] iptables.insert silently ignoring changes to to-destination value

Open
#58,696 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug severity-medium
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

Description
The iptables.insert state doesn't make the any changes to PREROUTING rules when to-destination changes. It reports that there are no changes required and does nothing.

Setup
I have a state that uses iptables.insert. With dependency information pruned and fake IP/host/port information substituted, it looks like this:

{%- set config = salt['pillar.get']('firewall:openvpn_with_mysql_forward') %}
{%- for db_config in config.get('databases', []) %}
{%- set db_host_ip = salt['dnsutil.A'](db_config.get('db_host_name'))[0] %}
# NAT incoming MYSQL traffic
nat_prerouting_tun_mysql_rule_for_{{ db_config.get('name') }}:
  # Insert to take precendence over older entries.
  iptables.insert:
    - table: nat
    - position: {{ loop.index }}
    - chain: PREROUTING
    - proto: tcp
    - dport: {{ db_config.get('dport', '3306') }}
    - in-interface: tun0
    - destination: {{ config.get('tun_host_ip') }}
    - jump: DNAT
    - to-destination: {{ db_host_ip }}:{{
        db_config.get('db_host_port', '3306')
      }}
    - save: True

Here is the associated pillar data

# salt-call pillar.get firewall:openvpn_with_mysql_forward
local:
    ----------
    databases:
        |_
          ----------
          db_host_name:
              some-rds-host.some-aws-hash.us-east-1.rds.amazonaws.com
          db_host_port:
              1234
          dport:
              3307
          name:
              mydb
    tun_host_ip:
        10.88.89.1
    tun_host_network:
        10.88.89.0/255.255.255.0

After rendering, it looks like this:

# salt-call state.show_sls firewall.openvpn-with-mysql-forward
[INFO    ] Loading fresh modules for state activity
[INFO    ] Executing command ['dig', '+short', 'some-rds-host.some-aws-hash.us-east-1.rds.amazonaws.com', 'A'] in directory '/root'
local:
    ----------

    nat_prerouting_tun_mysql_rule_for_mydb:
        ----------
        iptables:
            |_
              ----------
              table:
                  nat
            |_
              ----------
              position:
                  2
            |_
              ----------
              chain:
                  PREROUTING
            |_
              ----------
              proto:
                  tcp
            |_
              ----------
              dport:
                  3307
            |_
              ----------
              in-interface:
                  tun0
            |_
              ----------
              destination:
                  10.88.89.1
            |_
              ----------
              jump:
                  DNAT
            |_
              ----------
              to-destination:
                  1.2.3.4:1234
            |_
              ----------
              save:
                  True
            - insert
            |_
              ----------
              order:
                  10048
        __sls__:
            firewall.openvpn-with-mysql-forward
        __env__:
            base

The reason we use the salt['dnsutil.A'](db_config.get('db_host_name')) call is because sometimes the DB IP address will change and we'll need to re-run this state when a change is encountered.

The bug is that this doesn't actually do anything upon such changes.

eg. If some-rds-host.some-aws-hash.us-east-1.rds.amazonaws.com changed IPs from 3.4.5.6 to 1.2.3.4:

# iptables -t nat -L PREROUTING -nv
Chain PREROUTING (policy ACCEPT 2 packets, 120 bytes)
 pkts bytes target     prot opt in     out     source               destination         
    0     0 DNAT       tcp  --  tun0   *       0.0.0.0/0            10.88.89.1           tcp dpt:3306 to:9.8.7.6:3306
    2   120 DNAT       tcp  --  tun0   *       0.0.0.0/0            10.88.89.1           tcp dpt:3307 to:3.4.5.6:1234
#
# host some-rds-host.some-aws-hash.us-east-1.rds.amazonaws.com
some-rds-host.some-aws-hash.us-east-1.rds.amazonaws.com is an alias for ec2-1-2-3-4.us-east-1.compute.amazonaws.com.
ec2-1-2-3-4.us-east-1.compute.amazonaws.com has address 1.2.3.4
#
# sudo salt-call dnsutil.A some-rds-host.some-aws-hash.us-east-1.rds.amazonaws.com
[INFO    ] Executing command ['dig', '+short', 'some-rds-host.some-aws-hash.us-east-1.rds.amazonaws.com', 'A'] in directory '/root'
local:
    - 1.2.3.4
#
# salt-call state.sls firewall.openvpn-with-mysql-forward
----------
          ID: nat_prerouting_tun_mysql_rule_for_mydb
    Function: iptables.insert
      Result: True
     Comment: iptables rule for nat_prerouting_tun_mysql_rule_for_mydb already set for ipv4 (/usr/sbin/iptables --wait -t nat -I PREROUTING 2 -p tcp --dport 3307 --in-interface tun0 --destination 10.88.89.1 --jump DNAT --to-destination 1.2.3.4:1234)
     Started: 14:49:52.916900
    Duration: 83.394 ms
     Changes:   

Summary for local
-------------
Succeeded: 1
Failed:     0
-------------
Total states run:     1
Total run time:    1.229 s
#

No changes to the PREROUTING rule was made.

Steps to Reproduce the behavior
The above information should suffice. The same pillar and state information can be used. You can manually set db_host_ip to 3.4.5.6 or whatever for testing - no need to actually query a DNS server. I only included that to provide clarity that it is expected to change.

Expected behavior
I expected to see something like:

# iptables -t nat -L PREROUTING -nv
Chain PREROUTING (policy ACCEPT 2 packets, 120 bytes)
 pkts bytes target     prot opt in     out     source               destination         
    0     0 DNAT       tcp  --  tun0   *       0.0.0.0/0            10.88.89.1           tcp dpt:3306 to:9.8.7.6:3306
    2   120 DNAT       tcp  --  tun0   *       0.0.0.0/0            10.88.89.1           tcp dpt:3307 to:1.2.3.4:1234
    2   120 DNAT       tcp  --  tun0   *       0.0.0.0/0            10.88.89.1           tcp dpt:3307 to:3.4.5.6:1234
#

Note that the third line would be ignored here since it would never match, and is expected for this example. It would be cleaned up in another state block that is unrelated to the issue here.

Versions Report

salt --versions-report
Salt Version:
           Salt: 3001.1
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.7.3
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: Not Installed
   pycryptodome: 3.6.1
         pygit2: Not Installed
         Python: 3.7.3 (default, Jul 25 2020, 13:03:44)
   python-gnupg: 0.4.4
         PyYAML: 3.13
          PyZMQ: 17.1.2
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.5.3
            ZMQ: 4.3.1
 
System Versions:
           dist: debian 10 buster
         locale: UTF-8
        machine: x86_64
        release: 4.19.0-11-cloud-amd64
         system: Linux
        version: Debian GNU/Linux 10 buster

Master and minions are both running Debian GNU/Linux 10 (Buster) and the same Salt 3001.1 version from Salt's official 3001.1+ds-1 package.

Additional context
Add any other context about the problem here.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating the iptables.insert state implementation and reproduce the provided PREROUTING scenario with a changed to-destination value. Done means the existing rule is updated or the new destination is applied instead of reporting that the rule is already set, with coverage for this change-detection case.

Written by the indexing model from the issue text.

Assessment

Tech stack
linux, python
Domain
infrastructure, networking
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.