x/vmware-nsx
Code Issues Proposed changes

Add comments to iptables rules to help debugging

Browse Source 
Adds comments to some of the iptables rules generated
by neutron to assist with debugging.

DocImpact

Partial-Bug: #1265493
Change-Id: I7d9e37b9a43589dd7b142869b86fa15cb3fb329c
This commit is contained in:
Kevin Benton 2014-05-09 21:54:39 -07:00 committed by Kevin Benton
parent dd012a1ff7
commit 8ee0600a80
8 changed files with 585 additions and 220 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
etc/neutron.conf
View File

@ -545,6 +545,10 @@ lock_path = $state_path/lock
# Change to "sudo" to skip the filtering and just run the comand directly
# root_helper = sudo
# Set to true to add comments to generated iptables rules that describe
# each rule's purpose. (System must support the iptables comments module.)
# comment_iptables_rules = True
# =========== items for agent management extension =============
# seconds between nodes reporting state to server; should be less than
# agent_down_time, best if it is half or less than agent_down_time

9
neutron/agent/common/config.py
View File

@ -46,6 +46,11 @@ USE_NAMESPACES_OPTS = [
help=_("Allow overlapping IP.")),
]
IPTABLES_OPTS = [
cfg.BoolOpt('comment_iptables_rules', default=True,
help=_("Add comments to iptables rules.")),
]
def get_log_args(conf, log_file_name):
cmd_args = []
@ -92,6 +97,10 @@ def register_use_namespaces_opts_helper(conf):
conf.register_opts(USE_NAMESPACES_OPTS)
def register_iptables_opts(conf):
conf.register_opts(IPTABLES_OPTS, 'AGENT')
def get_root_helper(conf):
root_helper = conf.AGENT.root_helper
if root_helper != 'sudo':

34
neutron/agent/linux/iptables_comments.py Normal file
View File

@ -0,0 +1,34 @@
# Copyright 2014 OpenStack Foundation
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
"""iptables comments"""
# Do not translate these comments. These comments cannot contain a quote or
# an escape character because they will end up in a call to iptables and
# could interfere with other parameters.
SNAT_OUT = 'Perform source NAT on outgoing traffic.'
UNMATCH_DROP = 'Default drop rule for unmatched traffic.'
VM_INT_SG = 'Direct traffic from the VM interface to the security group chain.'
SG_TO_VM_SG = 'Jump to the VM specific chain.'
INPUT_TO_SG = 'Direct incoming traffic from VM to the security group chain.'
PAIR_ALLOW = 'Allow traffic from defined IP/MAC pairs.'
PAIR_DROP = 'Drop traffic without an IP/MAC allow rule.'
DHCP_CLIENT = 'Allow DHCP client traffic.'
DHCP_SPOOF = 'Prevent DHCP Spoofing by VM.'
UNMATCHED = 'Send unmatched traffic to the fallback chain.'
STATELESS_DROP = 'Drop packets that are not associated with a state.'
ALLOW_ASSOC = ('Direct packets associated with a known session to the RETURN '
'chain.')
IPV6_RA_ALLOW = 'Allow IPv6 ICMP traffic to allow RA packets.'

57
neutron/agent/linux/iptables_firewall.py
View File

@ -18,6 +18,7 @@ from oslo.config import cfg
from neutron.agent import firewall
from neutron.agent.linux import ipset_manager
from neutron.agent.linux import iptables_comments as ic
from neutron.agent.linux import iptables_manager
from neutron.common import constants
from neutron.common import ipv6_utils
@ -40,6 +41,7 @@ LINUX_DEV_LEN = 14
IPSET_CHAIN_LEN = 20
IPSET_CHANGE_BULK_THRESHOLD = 10
IPSET_ADD_BULK_THRESHOLD = 5
comment_rule = iptables_manager.comment_rule
class IptablesFirewallDriver(firewall.FirewallDriver):
@ -146,9 +148,11 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
def _add_fallback_chain_v4v6(self):
self.iptables.ipv4['filter'].add_chain('sg-fallback')
self.iptables.ipv4['filter'].add_rule('sg-fallback', '-j DROP')
self.iptables.ipv4['filter'].add_rule('sg-fallback', '-j DROP',
comment=ic.UNMATCH_DROP)
self.iptables.ipv6['filter'].add_chain('sg-fallback')
self.iptables.ipv6['filter'].add_rule('sg-fallback', '-j DROP')
self.iptables.ipv6['filter'].add_rule('sg-fallback', '-j DROP',
comment=ic.UNMATCH_DROP)
def _add_chain_by_name_v4v6(self, chain_name):
self.iptables.ipv6['filter'].add_chain(chain_name)
@ -158,12 +162,15 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
self.iptables.ipv4['filter'].ensure_remove_chain(chain_name)
self.iptables.ipv6['filter'].ensure_remove_chain(chain_name)
def _add_rule_to_chain_v4v6(self, chain_name, ipv4_rules, ipv6_rules):
def _add_rule_to_chain_v4v6(self, chain_name, ipv4_rules, ipv6_rules,
comment=None):
for rule in ipv4_rules:
self.iptables.ipv4['filter'].add_rule(chain_name, rule)
self.iptables.ipv4['filter'].add_rule(chain_name, rule,
comment=comment)
for rule in ipv6_rules:
self.iptables.ipv6['filter'].add_rule(chain_name, rule)
self.iptables.ipv6['filter'].add_rule(chain_name, rule,
comment=comment)
def _get_device_name(self, port):
return port['device']
@ -183,17 +190,20 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
'-j $%s' % (self.IPTABLES_DIRECTION[direction],
device,
SG_CHAIN)]
self._add_rule_to_chain_v4v6('FORWARD', jump_rule, jump_rule)
self._add_rule_to_chain_v4v6('FORWARD', jump_rule, jump_rule,
comment=ic.VM_INT_SG)
# jump to the chain based on the device
jump_rule = ['-m physdev --%s %s --physdev-is-bridged '
'-j $%s' % (self.IPTABLES_DIRECTION[direction],
device,
chain_name)]
self._add_rule_to_chain_v4v6(SG_CHAIN, jump_rule, jump_rule)
self._add_rule_to_chain_v4v6(SG_CHAIN, jump_rule, jump_rule,
comment=ic.SG_TO_VM_SG)
if direction == EGRESS_DIRECTION:
self._add_rule_to_chain_v4v6('INPUT', jump_rule, jump_rule)
self._add_rule_to_chain_v4v6('INPUT', jump_rule, jump_rule,
comment=ic.INPUT_TO_SG)
def _split_sgr_by_ethertype(self, security_group_rules):
ipv4_sg_rules = []
@ -222,12 +232,12 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
# of the list after the allowed_address_pair rules.
table.add_rule(chain_name,
'-m mac --mac-source %s -j RETURN'
% mac)
% mac, comment=ic.PAIR_ALLOW)
else:
table.add_rule(chain_name,
'-m mac --mac-source %s -s %s -j RETURN'
% (mac, ip))
table.add_rule(chain_name, '-j DROP')
% (mac, ip), comment=ic.PAIR_ALLOW)
table.add_rule(chain_name, '-j DROP', comment=ic.PAIR_DROP)
rules.append('-j $%s' % chain_name)
def _build_ipv4v6_mac_ip_list(self, mac, ip_address, mac_ipv4_pairs,
@ -239,9 +249,12 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
def _spoofing_rule(self, port, ipv4_rules, ipv6_rules):
#Note(nati) allow dhcp or RA packet
ipv4_rules += ['-p udp -m udp --sport 68 --dport 67 -j RETURN']
ipv6_rules += ['-p icmpv6 -j RETURN']
ipv6_rules += ['-p udp -m udp --sport 546 --dport 547 -j RETURN']
ipv4_rules += [comment_rule('-p udp -m udp --sport 68 --dport 67 '
'-j RETURN', comment=ic.DHCP_CLIENT)]
ipv6_rules += [comment_rule('-p icmpv6 -j RETURN',
comment=ic.IPV6_RA_ALLOW)]
ipv6_rules += [comment_rule('-p udp -m udp --sport 546 --dport 547 '
'-j RETURN', comment=None)]
mac_ipv4_pairs = []
mac_ipv6_pairs = []
@ -266,8 +279,10 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
def _drop_dhcp_rule(self, ipv4_rules, ipv6_rules):
#Note(nati) Drop dhcp packet from VM
ipv4_rules += ['-p udp -m udp --sport 67 --dport 68 -j DROP']
ipv6_rules += ['-p udp -m udp --sport 547 --dport 546 -j DROP']
ipv4_rules += [comment_rule('-p udp -m udp --sport 67 --dport 68 '
'-j DROP', comment=ic.DHCP_SPOOF)]
ipv6_rules += [comment_rule('-p udp -m udp --sport 547 --dport 546 '
'-j DROP', comment=None)]
def _accept_inbound_icmpv6(self):
# Allow multicast listener, neighbor solicitation and
@ -454,18 +469,22 @@ class IptablesFirewallDriver(firewall.FirewallDriver):
args += ['-j RETURN']
iptables_rules += [' '.join(args)]
iptables_rules += ['-j $sg-fallback']
iptables_rules += [comment_rule('-j $sg-fallback',
comment=ic.UNMATCHED)]
return iptables_rules
def _drop_invalid_packets(self, iptables_rules):
# Always drop invalid packets
iptables_rules += ['-m state --state ' 'INVALID -j DROP']
iptables_rules += [comment_rule('-m state --state ' 'INVALID -j DROP',
comment=ic.STATELESS_DROP)]
return iptables_rules
def _allow_established(self, iptables_rules):
# Allow established connections
iptables_rules += ['-m state --state RELATED,ESTABLISHED -j RETURN']
iptables_rules += [comment_rule(
'-m state --state RELATED,ESTABLISHED -j RETURN',
comment=ic.ALLOW_ASSOC)]
return iptables_rules
def _protocol_arg(self, protocol):

32
neutron/agent/linux/iptables_manager.py
View File

@ -22,6 +22,10 @@ import inspect
import os
import re
from oslo.config import cfg
from neutron.agent.common import config
from neutron.agent.linux import iptables_comments as ic
from neutron.agent.linux import utils as linux_utils
from neutron.common import utils
from neutron.openstack.common import excutils
@ -51,6 +55,12 @@ MAX_CHAIN_LEN_NOWRAP = 28
IPTABLES_ERROR_LINES_OF_CONTEXT = 5
def comment_rule(rule, comment):
if not cfg.CONF.AGENT.comment_iptables_rules or not comment:
return rule
return '%s -m comment --comment "%s"' % (rule, comment)
def get_chain_name(chain_name, wrap=True):
if wrap:
return chain_name[:MAX_CHAIN_LEN_WRAP]
@ -67,13 +77,14 @@ class IptablesRule(object):
"""
def __init__(self, chain, rule, wrap=True, top=False,
binary_name=binary_name, tag=None):
binary_name=binary_name, tag=None, comment=None):
self.chain = get_chain_name(chain, wrap)
self.rule = rule
self.wrap = wrap
self.top = top
self.wrap_name = binary_name[:16]
self.tag = tag
self.comment = comment
def __eq__(self, other):
return ((self.chain == other.chain) and
@ -89,7 +100,7 @@ class IptablesRule(object):
chain = '%s-%s' % (self.wrap_name, self.chain)
else:
chain = self.chain
return '-A %s %s' % (chain, self.rule)
return comment_rule('-A %s %s' % (chain, self.rule), self.comment)
class IptablesTable(object):
@ -182,7 +193,8 @@ class IptablesTable(object):
self.rules = [r for r in self.rules
if jump_snippet not in r.rule]
def add_rule(self, chain, rule, wrap=True, top=False, tag=None):
def add_rule(self, chain, rule, wrap=True, top=False, tag=None,
comment=None):
"""Add a rule to the table.
This is just like what you'd feed to iptables, just without
@ -202,7 +214,7 @@ class IptablesTable(object):
self._wrap_target_chain(e, wrap) for e in rule.split(' '))
self.rules.append(IptablesRule(chain, rule, wrap, top, self.wrap_name,
tag))
tag, comment))
def _wrap_target_chain(self, s, wrap):
if s.startswith('$'):
@ -210,7 +222,7 @@ class IptablesTable(object):
return s
def remove_rule(self, chain, rule, wrap=True, top=False):
def remove_rule(self, chain, rule, wrap=True, top=False, comment=None):
"""Remove a rule from a chain.
Note: The rule must be exactly identical to the one that was added.
@ -225,10 +237,12 @@ class IptablesTable(object):
self._wrap_target_chain(e, wrap) for e in rule.split(' '))
self.rules.remove(IptablesRule(chain, rule, wrap, top,
self.wrap_name))
self.wrap_name,
comment=comment))
if not wrap:
self.remove_rules.append(IptablesRule(chain, rule, wrap, top,
self.wrap_name))
self.wrap_name,
comment=comment))
except ValueError:
LOG.warn(_('Tried to remove rule that was not there:'
' %(chain)r %(rule)r %(wrap)r %(top)r'),
@ -288,6 +302,7 @@ class IptablesManager(object):
else:
self.execute = linux_utils.execute
config.register_iptables_opts(cfg.CONF)
self.use_ipv6 = use_ipv6
self.root_helper = root_helper
self.namespace = namespace
@ -351,7 +366,8 @@ class IptablesManager(object):
# chain so that it's applied last.
self.ipv4['nat'].add_chain('snat')
self.ipv4['nat'].add_rule('neutron-postrouting-bottom',
'-j $snat', wrap=False)
'-j $snat', wrap=False,
comment=ic.SNAT_OUT)
# And then we add a float-snat chain and jump to first thing in
# the snat chain.

556
neutron/tests/unit/test_iptables_firewall.py
View File

File diff suppressed because it is too large Load Diff

111
neutron/tests/unit/test_iptables_manager.py
View File

@ -17,13 +17,17 @@ import inspect
import os
import mock
from oslo.config import cfg
from neutron.agent.common import config as a_cfg
from neutron.agent.linux import iptables_comments as ic
from neutron.agent.linux import iptables_manager
from neutron.tests import base
from neutron.tests import tools
IPTABLES_ARG = {'bn': iptables_manager.binary_name}
IPTABLES_ARG = {'bn': iptables_manager.binary_name,
'snat_out_comment': ic.SNAT_OUT}
NAT_DUMP = ('# Generated by iptables_manager\n'
'*nat\n'
@ -59,6 +63,107 @@ FILTER_DUMP = ('# Generated by iptables_manager\n'
'COMMIT\n'
'# Completed by iptables_manager\n' % IPTABLES_ARG)
COMMENTED_NAT_DUMP = (
'# Generated by iptables_manager\n'
'*nat\n'
':neutron-postrouting-bottom - [0:0]\n'
':%(bn)s-OUTPUT - [0:0]\n'
':%(bn)s-POSTROUTING - [0:0]\n'
':%(bn)s-PREROUTING - [0:0]\n'
':%(bn)s-float-snat - [0:0]\n'
':%(bn)s-snat - [0:0]\n'
'[0:0] -A PREROUTING -j %(bn)s-PREROUTING\n'
'[0:0] -A OUTPUT -j %(bn)s-OUTPUT\n'
'[0:0] -A POSTROUTING -j %(bn)s-POSTROUTING\n'
'[0:0] -A POSTROUTING -j neutron-postrouting-bottom\n'
'[0:0] -A neutron-postrouting-bottom -j %(bn)s-snat '
'-m comment --comment "%(snat_out_comment)s"\n'
'[0:0] -A %(bn)s-snat -j '
'%(bn)s-float-snat\n'
'COMMIT\n'
'# Completed by iptables_manager\n' % IPTABLES_ARG)
class IptablesCommentsTestCase(base.BaseTestCase):
def setUp(self):
super(IptablesCommentsTestCase, self).setUp()
cfg.CONF.register_opts(a_cfg.IPTABLES_OPTS, 'AGENT')
cfg.CONF.set_override('comment_iptables_rules', True, 'AGENT')
self.root_helper = 'sudo'
self.iptables = (iptables_manager.
IptablesManager(root_helper=self.root_helper))
self.execute = mock.patch.object(self.iptables, "execute").start()
def test_comments_short_enough(self):
for attr in dir(ic):
if not attr.startswith('__') and len(getattr(ic, attr)) > 255:
self.fail("Iptables comment %s is longer than 255 characters."
% attr)
def test_add_filter_rule(self):
filter_dump_mod = ('# Generated by iptables_manager\n'
'*filter\n'
':neutron-filter-top - [0:0]\n'
':%(bn)s-FORWARD - [0:0]\n'
':%(bn)s-INPUT - [0:0]\n'
':%(bn)s-OUTPUT - [0:0]\n'
':%(bn)s-filter - [0:0]\n'
':%(bn)s-local - [0:0]\n'
'[0:0] -A FORWARD -j neutron-filter-top\n'
'[0:0] -A OUTPUT -j neutron-filter-top\n'
'[0:0] -A neutron-filter-top -j %(bn)s-local\n'
'[0:0] -A INPUT -j %(bn)s-INPUT\n'
'[0:0] -A OUTPUT -j %(bn)s-OUTPUT\n'
'[0:0] -A FORWARD -j %(bn)s-FORWARD\n'
'[0:0] -A %(bn)s-filter -j DROP\n'
'[0:0] -A %(bn)s-INPUT -s 0/0 -d 192.168.0.2 -j '
'%(bn)s-filter\n'
'COMMIT\n'
'# Completed by iptables_manager\n'
% IPTABLES_ARG)
raw_dump = _generate_raw_dump(IPTABLES_ARG)
expected_calls_and_values = [
(mock.call(['iptables-save', '-c'],
root_helper=self.root_helper),
''),
(mock.call(['iptables-restore', '-c'],
process_input=(
raw_dump + COMMENTED_NAT_DUMP + filter_dump_mod),
root_helper=self.root_helper),
None),
(mock.call(['iptables-save', '-c'],
root_helper=self.root_helper),
''),
(mock.call(['iptables-restore', '-c'],
process_input=(
raw_dump + COMMENTED_NAT_DUMP + FILTER_DUMP),
root_helper=self.root_helper
),
None),
]
tools.setup_mock_calls(self.execute, expected_calls_and_values)
self.iptables.ipv4['filter'].add_chain('filter')
self.iptables.ipv4['filter'].add_rule('filter', '-j DROP')
self.iptables.ipv4['filter'].add_rule('INPUT',
'-s 0/0 -d 192.168.0.2 -j'
' %(bn)s-filter' % IPTABLES_ARG)
self.iptables.apply()
self.iptables.ipv4['filter'].remove_rule('filter', '-j DROP')
self.iptables.ipv4['filter'].remove_rule('INPUT',
'-s 0/0 -d 192.168.0.2 -j'
' %(bn)s-filter'
% IPTABLES_ARG)
self.iptables.ipv4['filter'].remove_chain('filter')
self.iptables.apply()
tools.verify_mock_calls(self.execute, expected_calls_and_values)
def _generate_raw_dump(iptables_args):
return ('# Generated by iptables_manager\n'
@ -77,6 +182,8 @@ class IptablesManagerStateFulTestCase(base.BaseTestCase):
def setUp(self):
super(IptablesManagerStateFulTestCase, self).setUp()
cfg.CONF.register_opts(a_cfg.IPTABLES_OPTS, 'AGENT')
cfg.CONF.set_override('comment_iptables_rules', False, 'AGENT')
self.root_helper = 'sudo'
self.iptables = iptables_manager.IptablesManager(
root_helper=self.root_helper)
@ -936,6 +1043,8 @@ class IptablesManagerStateLessTestCase(base.BaseTestCase):
def setUp(self):
super(IptablesManagerStateLessTestCase, self).setUp()
cfg.CONF.register_opts(a_cfg.IPTABLES_OPTS, 'AGENT')
cfg.CONF.set_override('comment_iptables_rules', False, 'AGENT')
self.iptables = (iptables_manager.IptablesManager(state_less=True))
def test_nat_not_found(self):

2
neutron/tests/unit/test_security_groups_rpc.py
View File

@ -2254,11 +2254,13 @@ class TestSecurityGroupAgentWithIptables(base.BaseTestCase):
def setUp(self, defer_refresh_firewall=False, test_rpc_v1_1=True):
super(TestSecurityGroupAgentWithIptables, self).setUp()
config.register_root_helper(cfg.CONF)
config.register_iptables_opts(cfg.CONF)
cfg.CONF.set_override(
'lock_path',
'$state_path/lock')
set_firewall_driver(self.FIREWALL_DRIVER)
cfg.CONF.set_override('enable_ipset', False, group='SECURITYGROUP')
cfg.CONF.set_override('comment_iptables_rules', False, group='AGENT')
self.agent = sg_rpc.SecurityGroupAgentRpcMixin()
self.agent.context = None