Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support proto for IPSec policy extension in iptables state #47113

Merged
merged 1 commit into from Apr 25, 2018

Conversation

jfindlay
Copy link
Contributor

@jfindlay jfindlay commented Apr 17, 2018

What does this PR do?

Support --proto for iptables IPSec policy extension in iptables state.

What issues does this PR fix or reference?

I found no issues (open or closed) referencing this problem.

Previous Behavior

Consider the following state:

ipt-test:
  iptables.append:
    - name: ipt-test
    - table: filter
    - chain: INPUT
    - source: 10.20.0.0/24
    - destination: 10.10.0.0/24
    - in-interface: eth0
    - match: policy
    - proto: esp
    - dir: in
    - pol: ipsec
    - reqid: 1
    - jump: ACCEPT

Prior to this fix, applying ipt-test would produce this incorrect iptables rule:

# iptables -vL
...
 pkts bytes target     prot opt in     out     source               destination
...
    0     0 ACCEPT     esp  --  eth0   any     10.20.0.0/24         10.10.0.0/24         policy match dir in pol ipsec reqid 1
...
# iptables-save
...
-A INPUT -s 10.20.0.0/24 -d 10.10.0.0/24 -i eth0 -p esp -m policy --dir in --pol ipsec --reqid 1 -j ACCEPT
...

New Behavior

The state ipt-test now produces the correct iptables rule:

# iptables -vL
...
 pkts bytes target     prot opt in     out     source               destination
...
    0     0 ACCEPT     all  --  eth0   any     10.20.0.0/24         10.10.0.0/24         policy match dir in pol ipsec reqid 1 proto esp
...
# iptables-save
...
-A INPUT -s 10.20.0.0/24 -d 10.10.0.0/24 -i eth0 -m policy --dir in --pol ipsec --reqid 1 --proto esp -j ACCEPT
...

Notice how the proto is now interpreted as a parameter of the IPSec policy extension of iptables.

Tests written?

Automating tests for this would require iptables to be installed and working on the test system. If requested, I will attempt to make the necessary changes to SaltTesting. The full test case I used is copied below. The details of the state failure are shown in the Previous Behavior section. I compared the rules generated by the execution and state modules before and after the change against the rules generated with the raw iptables commands.

Raw commands

iptables -A INPUT -s 10.20.0.0/24 -d 10.10.0.0/24 -i eth0 -m policy --protocol esp --dir in --pol ipsec --reqid 1 -j ACCEPT
iptables -A INPUT --proto esp -s 10.20.0.0/24 -d 10.10.0.0/24 -i eth0 -m policy --proto esp --dir in --pol ipsec --reqid 1 -j ACCEPT
iptables -A INPUT -s 10.20.0.0/24 -d 10.10.0.0/24 -i eth0 -m policy --proto esp --dir in --pol ipsec --reqid 1 -j ACCEPT

salt iptables.append command line

salt-call --local iptables.append filter INPUT rule='-s 10.20.0.0/24 -d 10.10.0.0/24 -i eth0 -m policy --protocol esp --dir in --pol ipsec --reqid 1 -j ACCEPT'
salt-call --local iptables.append filter INPUT rule='--proto esp -s 10.20.0.0/24 -d 10.10.0.0/24 -i eth0 -m policy --proto esp --dir in --pol ipsec --reqid 1 -j ACCEPT'
salt-call --local iptables.append filter INPUT rule='-s 10.20.0.0/24 -d 10.10.0.0/24 -i eth0 -m policy --proto esp --dir in --pol ipsec --reqid 1 -j ACCEPT'

salt state.single iptables.append command line

salt-call --local state.single iptables.append name=ipt-test table=filter chain=INPUT source=10.20.0.0/24 destination=10.10.0.0/24 in-interface=eth0 match=policy protocol=esp dir=in pol=ipsec reqid=1 jump=ACCEPT
salt-call --local state.single iptables.append name=ipt-test table=filter chain=INPUT protocol=esp source=10.20.0.0/24 destination=10.10.0.0/24 in-interface=eth0 match=policy proto=esp dir=in pol=ipsec reqid=1 jump=ACCEPT
salt-call --local state.single iptables.append name=ipt-test table=filter chain=INPUT source=10.20.0.0/24 destination=10.10.0.0/24 in-interface=eth0 match=policy proto=esp dir=in pol=ipsec reqid=1 jump=ACCEPT

iptables-save output

-A INPUT -s 10.20.0.0/24 -d 10.10.0.0/24 -i eth0 -p esp -m policy --dir in --pol ipsec --reqid 1 -j ACCEPT
-A INPUT -s 10.20.0.0/24 -d 10.10.0.0/24 -i eth0 -p esp -m policy --dir in --pol ipsec --reqid 1 --proto esp -j ACCEPT
-A INPUT -s 10.20.0.0/24 -d 10.10.0.0/24 -i eth0 -m policy --dir in --pol ipsec --reqid 1 --proto esp -j ACCEPT

iptables -vL output

    0     0 ACCEPT     esp  --  eth0   any     10.20.0.0/24         10.10.0.0/24         policy match dir in pol ipsec reqid 1
    0     0 ACCEPT     esp  --  eth0   any     10.20.0.0/24         10.10.0.0/24         policy match dir in pol ipsec reqid 1 proto esp
    0     0 ACCEPT     all  --  eth0   any     10.20.0.0/24         10.10.0.0/24         policy match dir in pol ipsec reqid 1 proto esp

Results

Mode Before After
exec module pass pass
state module fail pass

Commits signed with GPG?

Yes

@jfindlay jfindlay changed the title modules,states.iptables support proto for policy ext Support proto for IPSec policy extension in iptables state Apr 17, 2018
Copy link
Contributor

@gtmanfred gtmanfred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good other than the one small change.

@@ -289,6 +299,9 @@ def maybe_add_negation(arg):
if 'name_' in kwargs and match.strip() in ('pknock', 'quota2', 'recent'):
rule.append('--name {0}'.format(kwargs['name_']))
del kwargs['name_']
if 'proto' in kwargs and kwargs['match'] == 'policy':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably also be kwargs.get('match')

@rallytime rallytime merged commit 44f19b2 into saltstack:2017.7 Apr 25, 2018
@jfindlay jfindlay deleted the iptables_state branch April 30, 2018 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants