From 5b8ff27d98ec90cb7582c1fdf0e1785860638b48 Mon Sep 17 00:00:00 2001 From: Akihiro MOTOKI Date: Fri, 1 Mar 2013 07:19:20 +0900 Subject: [PATCH] Ensure max length of iptables chain name w/o prefix is up to 11 chars. The maximum length of Linux iptables chain name must be less than or equal to 28 characters. In iptables_manager binary_name up to 16 chars is used as a prefix and a '-' follows it, so a chain name passed to iptables_manager must be less than 12 character long. Accordingky MAX_CHAIN_LEN should be changed from 28 to 12. Also this commit introduces a method to get a chain name with valid length. Since iptables_firewall module constructs a rule by directly using a chain name, iptable_firewall also must take care of the length. Fixes bug #1133833 Change-Id: I6157d519f3cb91ec32dc6a92eae45439b8717b2d --- quantum/agent/linux/iptables_firewall.py | 8 +++--- quantum/agent/linux/iptables_manager.py | 28 +++++++++++++++------ quantum/tests/unit/test_iptables_manager.py | 10 ++++++++ 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/quantum/agent/linux/iptables_firewall.py b/quantum/agent/linux/iptables_firewall.py index df26369cbb..83c65f08b9 100644 --- a/quantum/agent/linux/iptables_firewall.py +++ b/quantum/agent/linux/iptables_firewall.py @@ -274,8 +274,8 @@ class IptablesFirewallDriver(firewall.FirewallDriver): return [] def _port_chain_name(self, port, direction): - return '%s%s' % (CHAIN_NAME_PREFIX[direction], - port['device'][3:]) + return iptables_manager.get_chain_name( + '%s%s' % (CHAIN_NAME_PREFIX[direction], port['device'][3:])) def filter_defer_apply_on(self): self.iptables.defer_apply_on() @@ -288,8 +288,8 @@ class OVSHybridIptablesFirewallDriver(IptablesFirewallDriver): OVS_HYBRID_TAP_PREFIX = 'tap' def _port_chain_name(self, port, direction): - return '%s%s' % (CHAIN_NAME_PREFIX[direction], - port['device']) + return iptables_manager.get_chain_name( + '%s%s' % (CHAIN_NAME_PREFIX[direction], port['device'])) def _get_device_name(self, port): return (self.OVS_HYBRID_TAP_PREFIX + port['device'])[:LINUX_DEV_LEN] diff --git a/quantum/agent/linux/iptables_manager.py b/quantum/agent/linux/iptables_manager.py index b7b4eca5a2..4d6de26573 100644 --- a/quantum/agent/linux/iptables_manager.py +++ b/quantum/agent/linux/iptables_manager.py @@ -36,8 +36,19 @@ LOG = logging.getLogger(__name__) # so we limit it to 16 characters. # (max_chain_name_length - len('-POSTROUTING') == 16) binary_name = os.path.basename(inspect.stack()[-1][1])[:16] +# A length of a chain name must be less than or equal to 11 characters. +# - ( + '-') = 28-(16+1) = 11 +MAX_CHAIN_LEN_WRAP = 11 +MAX_CHAIN_LEN_NOWRAP = 28 + cfg.CONF.set_default('lock_path', '$state_path/lock') -MAX_CHAIN_LEN = 28 + + +def get_chain_name(chain_name, wrap=True): + if wrap: + return chain_name[:MAX_CHAIN_LEN_WRAP] + else: + return chain_name[:MAX_CHAIN_LEN_NOWRAP] class IptablesRule(object): @@ -49,7 +60,7 @@ class IptablesRule(object): """ def __init__(self, chain, rule, wrap=True, top=False): - self.chain = chain[:MAX_CHAIN_LEN] + self.chain = get_chain_name(chain, wrap) self.rule = rule self.wrap = wrap self.top = top @@ -68,7 +79,6 @@ class IptablesRule(object): chain = '%s-%s' % (binary_name, self.chain) else: chain = self.chain - chain = chain[:MAX_CHAIN_LEN] return '-A %s %s' % (chain, self.rule) @@ -92,7 +102,7 @@ class IptablesTable(object): end up named 'nova-compute-OUTPUT'. """ - name = name[:MAX_CHAIN_LEN] + name = get_chain_name(name, wrap) if wrap: self.chains.add(name) else: @@ -110,7 +120,7 @@ class IptablesTable(object): This removal "cascades". All rule in the chain are removed, as are all rules in other chains that jump to it. """ - name = name[:MAX_CHAIN_LEN] + name = get_chain_name(name, wrap) chain_set = self._select_chain_set(wrap) if name not in chain_set: return @@ -126,7 +136,7 @@ class IptablesTable(object): If the chain is not found, this is merely logged. """ - name = name[:MAX_CHAIN_LEN] + name = get_chain_name(name, wrap) chain_set = self._select_chain_set(wrap) if name not in chain_set: @@ -154,6 +164,7 @@ class IptablesTable(object): is applied correctly. """ + chain = get_chain_name(chain, wrap) if wrap and chain not in self.chains: raise LookupError(_('Unknown chain: %r') % chain) @@ -164,7 +175,7 @@ class IptablesTable(object): def _wrap_target_chain(self, s): if s.startswith('$'): - return ('%s-%s' % (binary_name, s[1:]))[:MAX_CHAIN_LEN] + return ('%s-%s' % (binary_name, s[1:])) return s def remove_rule(self, chain, rule, wrap=True, top=False): @@ -175,6 +186,7 @@ class IptablesTable(object): CLI tool. """ + chain = get_chain_name(chain, wrap) try: self.rules.remove(IptablesRule(chain, rule, wrap, top)) except ValueError: @@ -185,7 +197,7 @@ class IptablesTable(object): def empty_chain(self, chain, wrap=True): """Remove all rules from a chain.""" - chain = chain[:MAX_CHAIN_LEN] + chain = get_chain_name(chain, wrap) chained_rules = [rule for rule in self.rules if rule.chain == chain and rule.wrap == wrap] for rule in chained_rules: diff --git a/quantum/tests/unit/test_iptables_manager.py b/quantum/tests/unit/test_iptables_manager.py index cc72cd05cb..e8efa920f7 100644 --- a/quantum/tests/unit/test_iptables_manager.py +++ b/quantum/tests/unit/test_iptables_manager.py @@ -41,6 +41,16 @@ class IptablesManagerStateFulTestCase(testtools.TestCase): self.assertEqual(iptables_manager.binary_name, os.path.basename(inspect.stack()[-1][1])[:16]) + def test_get_chanin_name(self): + name = '0123456789' * 5 + # 28 chars is the maximum length of iptables chain name. + self.assertEqual(iptables_manager.get_chain_name(name, wrap=False), + name[:28]) + # 11 chars is the maximum length of chain name of iptable_manager + # if binary_name is prepended. + self.assertEqual(iptables_manager.get_chain_name(name, wrap=True), + name[:11]) + def test_add_and_remove_chain(self): bn = iptables_manager.binary_name self.iptables.execute(['iptables-save', '-t', 'filter'],