Add comments to iptables rules to help debugging

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 48ec1fbfc3
commit ea202faec9
8 changed files with 585 additions and 220 deletions

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

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':

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.'

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):

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.

File diff suppressed because it is too large Load Diff

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):

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