From e83a44b96a8e3cd81b7cc684ac90486b283a3507 Mon Sep 17 00:00:00 2001 From: Ihar Hrachyshka Date: Thu, 15 Sep 2016 21:48:10 +0000 Subject: [PATCH] iptables: fail to start ovs/linuxbridge agents on missing sysctl knobs For new kernels (3.18+), bridge module is split into two pieces: bridge and br_netfilter. The latter provides firewall support for bridged traffic, as well as the following sysctl knobs: * net.bridge.bridge-nf-call-arptables * net.bridge.bridge-nf-call-ip6tables * net.bridge.bridge-nf-call-iptables Before kernel 3.18, any brctl command was loading the 'bridge' module with the knobs, so at the moment where we reached iptables setup, they were always available. With new 3.18+ kernels, brctl still loads 'bridge' module, but not br_netfilter. So bridge existance no longer guarantees us knobs' presence. If we reach _enable_netfilter_for_bridges before the new module is loaded, then the code will fail, triggering agent resync. It will also fail to enable bridge firewalling on systems where it's disabled by default (examples of those systems are most if not all Red Hat/Fedora based systems), making security groups completely ineffective. Systems that don't override default settings for those knobs would work fine except for this exception in the log file and agent resync. This is because the first attempt to add a iptables rule using 'physdev' module (-m physdev) will trigger the kernel module loading. In theory, we could silently swallow missing knobs, and still operate correctly. But on second thought, it's quite fragile to rely on that implicit module loading. In the case where we can't detect whether firewall is enabled, it's better to fail than hope for the best. An alternative to the proposed path could be trying to fix broken deployment, meaning we would need to load the missing kernel module on agent startup. It's not even clear whether we can assume the operation would be available to us. Even with that, adding a rootwrap filter to allow loading code in the kernel sounds quite scary. If we would follow the path, we would also hit an issue of distinguishing between cases of built-in kernel module vs. modular one. A complexity that is probably beyond what Neutron should fix. The patch introduces a sanity check that would fail on missing configuration knobs. DocImpact: document the new deployment requirement in operations guide UpgradeImpact: deployers relying on agents fixing wrong sysctl defaults will need to make sure bridge firewalling is enabled. Also, the kernel module providing sysctl knobs must be loaded before starting the agent, otherwise it will fail to start. Depends-On: Id6bfd9595f0772a63d1096ef83ebbb6cd630fafd Change-Id: I9137ea017624ac92a05f73863b77f9ee4681bbe7 Related-Bug: #1622914 --- .../rootwrap.d/iptables-firewall.filters | 9 +++--- neutron/agent/linux/iptables_firewall.py | 28 ++++++++++++------- neutron/cmd/sanity/checks.py | 15 ++++++++++ neutron/cmd/sanity_check.py | 22 +++++++++++++++ .../tests/functional/sanity/test_sanity.py | 3 ++ .../agent/linux/test_iptables_firewall.py | 2 ++ .../unit/agent/test_securitygroups_rpc.py | 10 +++---- ...l-bridge-firewalling-912f157b5671363f.yaml | 14 ++++++++++ 8 files changed, 82 insertions(+), 21 deletions(-) create mode 100644 releasenotes/notes/iptables-fail-on-missing-sysctl-bridge-firewalling-912f157b5671363f.yaml diff --git a/etc/neutron/rootwrap.d/iptables-firewall.filters b/etc/neutron/rootwrap.d/iptables-firewall.filters index 29c78dae3f0..0a81f9ddb48 100644 --- a/etc/neutron/rootwrap.d/iptables-firewall.filters +++ b/etc/neutron/rootwrap.d/iptables-firewall.filters @@ -8,21 +8,20 @@ [Filters] -# neutron/agent/linux/iptables_manager.py +# neutron/agent/linux/iptables_firewall.py # "iptables-save", ... iptables-save: CommandFilter, iptables-save, root iptables-restore: CommandFilter, iptables-restore, root ip6tables-save: CommandFilter, ip6tables-save, root ip6tables-restore: CommandFilter, ip6tables-restore, root -# neutron/agent/linux/iptables_manager.py +# neutron/agent/linux/iptables_firewall.py # "iptables", "-A", ... iptables: CommandFilter, iptables, root ip6tables: CommandFilter, ip6tables, root -# neutron/agent/linux/iptables_manager.py -# "sysctl", "-w", ... +# neutron/agent/linux/iptables_firewall.py sysctl: CommandFilter, sysctl, root # neutron/agent/linux/ip_conntrack.py -conntrack: CommandFilter, conntrack, root \ No newline at end of file +conntrack: CommandFilter, conntrack, root diff --git a/neutron/agent/linux/iptables_firewall.py b/neutron/agent/linux/iptables_firewall.py index 8978de7c0dc..bd9b7bfaad1 100644 --- a/neutron/agent/linux/iptables_firewall.py +++ b/neutron/agent/linux/iptables_firewall.py @@ -20,10 +20,11 @@ import netaddr from neutron_lib import constants from oslo_config import cfg from oslo_log import log as logging +from oslo_log import versionutils from oslo_utils import netutils import six -from neutron._i18n import _LI +from neutron._i18n import _, _LI, _LW from neutron.agent import firewall from neutron.agent.linux import ip_conntrack from neutron.agent.linux import ipset_manager @@ -109,15 +110,22 @@ class IptablesFirewallDriver(firewall.FirewallDriver): # enabled by default or not (Ubuntu - yes, Redhat - no, for # example). LOG.debug("Enabling netfilter for bridges") - utils.execute(['sysctl', '-w', - 'net.bridge.bridge-nf-call-arptables=1'], - run_as_root=True) - utils.execute(['sysctl', '-w', - 'net.bridge.bridge-nf-call-ip6tables=1'], - run_as_root=True) - utils.execute(['sysctl', '-w', - 'net.bridge.bridge-nf-call-iptables=1'], - run_as_root=True) + entries = utils.execute(['sysctl', '-N', 'net.bridge'], + run_as_root=True).splitlines() + for proto in ('arp', 'ip', 'ip6'): + knob = 'net.bridge.bridge-nf-call-%stables' % proto + if 'net.bridge.bridge-nf-call-%stables' % proto not in entries: + raise SystemExit( + _("sysctl value %s not present on this system.") % knob) + enabled = utils.execute(['sysctl', '-b', knob]) + if enabled != '1': + versionutils.report_deprecated_feature( + LOG, + _LW('Bridge firewalling is disabled; enabling to make ' + 'iptables firewall work. This may not work in future ' + 'releases.')) + utils.execute( + ['sysctl', '-w', '%s=1' % knob], run_as_root=True) @property def ports(self): diff --git a/neutron/cmd/sanity/checks.py b/neutron/cmd/sanity/checks.py index 9c55080839d..da34ce411e2 100644 --- a/neutron/cmd/sanity/checks.py +++ b/neutron/cmd/sanity/checks.py @@ -228,6 +228,21 @@ def dhcp_release6_supported(): return True +def bridge_firewalling_enabled(): + for proto in ('arp', 'ip', 'ip6'): + knob = 'net.bridge.bridge-nf-call-%stables' % proto + cmd = ['sysctl', '-b', knob] + try: + out = agent_utils.execute(cmd) + except (OSError, RuntimeError, IndexError, ValueError) as e: + LOG.debug("Exception while extracting %(knob)s. " + "Exception: %(e)s", {'knob': knob, 'e': e}) + return False + if out == '0': + return False + return True + + class KeepalivedIPv6Test(object): def __init__(self, ha_port, gw_port, gw_vip, default_gw): self.ha_port = ha_port diff --git a/neutron/cmd/sanity_check.py b/neutron/cmd/sanity_check.py index 41ad5328dbb..8b2bd601766 100644 --- a/neutron/cmd/sanity_check.py +++ b/neutron/cmd/sanity_check.py @@ -256,6 +256,16 @@ def check_dhcp_release6(): return result +def check_bridge_firewalling_enabled(): + result = checks.bridge_firewalling_enabled() + if not result: + LOG.error(_LE('Bridge firewalling is not enabled. It may be the case ' + 'that bridge and/or br_netfilter kernel modules are not ' + 'loaded. Alternatively, corresponding sysctl settings ' + 'may be overridden to disable it by default.')) + return result + + # Define CLI opts to test specific features, with a callback for the test OPTS = [ BoolOptCallback('ovs_vxlan', check_ovs_vxlan, default=False, @@ -298,6 +308,9 @@ OPTS = [ help=_('Check ip6tables installation')), BoolOptCallback('dhcp_release6', check_dhcp_release6, help=_('Check dhcp_release6 installation')), + BoolOptCallback('bridge_firewalling', check_bridge_firewalling_enabled, + help=_('Check bridge firewalling'), + default=False), ] @@ -343,6 +356,15 @@ def enable_tests_from_config(): if ('sriovnicswitch' in cfg.CONF.ml2.mechanism_drivers and 'qos' in cfg.CONF.ml2.extension_drivers): cfg.CONF.set_default('vf_extended_management', True) + if cfg.CONF.SECURITYGROUP.firewall_driver in ( + 'iptables', + 'iptables_hybrid', + ('neutron.agent.linux.iptables_firewall.' + 'IptablesFirewallDriver'), + ('neutron.agent.linux.iptables_firewall.' + 'OVSHybridIptablesFirewallDriver'), + ): + cfg.CONF.set_default('bridge_firewalling', True) def all_tests_passed(): diff --git a/neutron/tests/functional/sanity/test_sanity.py b/neutron/tests/functional/sanity/test_sanity.py index e214558d400..76b31e3f3d7 100644 --- a/neutron/tests/functional/sanity/test_sanity.py +++ b/neutron/tests/functional/sanity/test_sanity.py @@ -85,3 +85,6 @@ class SanityTestCaseRoot(functional_base.BaseSudoTestCase): def test_keepalived_ipv6_support(self): checks.keepalived_ipv6_supported() + + def test_bridge_firewalling_enabled(self): + checks.bridge_firewalling_enabled() diff --git a/neutron/tests/unit/agent/linux/test_iptables_firewall.py b/neutron/tests/unit/agent/linux/test_iptables_firewall.py index ce3ad418a34..44c1e68f2d0 100644 --- a/neutron/tests/unit/agent/linux/test_iptables_firewall.py +++ b/neutron/tests/unit/agent/linux/test_iptables_firewall.py @@ -94,6 +94,8 @@ class BaseIptablesFirewallTestCase(base.BaseTestCase): RAW_TABLE_OUTPUT.splitlines()) self.firewall = iptables_firewall.IptablesFirewallDriver() self.firewall.iptables = self.iptables_inst + # don't mess with sysctl knobs in unit tests + self.firewall._enabled_netfilter_for_bridges = True class IptablesFirewallTestCase(BaseIptablesFirewallTestCase): diff --git a/neutron/tests/unit/agent/test_securitygroups_rpc.py b/neutron/tests/unit/agent/test_securitygroups_rpc.py index aecde3b5f47..33127456604 100644 --- a/neutron/tests/unit/agent/test_securitygroups_rpc.py +++ b/neutron/tests/unit/agent/test_securitygroups_rpc.py @@ -2746,6 +2746,8 @@ class TestSecurityGroupAgentWithIptables(base.BaseTestCase): context=None, plugin_rpc=self.rpc, defer_refresh_firewall=defer_refresh_firewall) self._enforce_order_in_firewall(self.agent.firewall) + # don't mess with sysctl knobs in unit tests + self.agent.firewall._enabled_netfilter_for_bridges = True def _device(self, device, ip, mac_address, rule): return {'device': device, @@ -2794,12 +2796,6 @@ class TestSecurityGroupAgentWithIptables(base.BaseTestCase): self.assertThat(kwargs['process_input'], matchers.MatchesRegex(expected_regex)) - expected = ['net.bridge.bridge-nf-call-arptables=1', - 'net.bridge.bridge-nf-call-ip6tables=1', - 'net.bridge.bridge-nf-call-iptables=1'] - for e in expected: - self.utils_exec.assert_any_call(['sysctl', '-w', e], - run_as_root=True) self.assertEqual(exp_fw_sg_updated_call, self.agent.firewall.security_group_updated.called) @@ -3123,6 +3119,8 @@ class TestSecurityGroupAgentWithOVSIptables( context=None, plugin_rpc=self.rpc, defer_refresh_firewall=defer_refresh_firewall) self._enforce_order_in_firewall(self.agent.firewall) + # don't mess with sysctl knobs in unit tests + self.agent.firewall._enabled_netfilter_for_bridges = True def test_prepare_remove_port(self): self.rpc.security_group_rules_for_devices.return_value = self.devices1 diff --git a/releasenotes/notes/iptables-fail-on-missing-sysctl-bridge-firewalling-912f157b5671363f.yaml b/releasenotes/notes/iptables-fail-on-missing-sysctl-bridge-firewalling-912f157b5671363f.yaml new file mode 100644 index 00000000000..d898e15b922 --- /dev/null +++ b/releasenotes/notes/iptables-fail-on-missing-sysctl-bridge-firewalling-912f157b5671363f.yaml @@ -0,0 +1,14 @@ +--- +deprecations: + - The iptables firewall driver will no longer enable bridge firewalling in + next versions of Neutron. If your distribution overrides the default + value for any of relevant sysctl settings + (``net.bridge.bridge-nf-call-arptables``, + ``net.bridge.bridge-nf-call-ip6tables``, and + ``net.bridge.bridge-nf-call-iptables``) then make sure you set them back + to upstream kernel default (``1``) using /etc/sysctl.conf or + /etc/sysctl.d/* configuration files. +upgrades: + - On newer Linux kernels (3.18+) you will need to load the ``br_netfilter`` + kernel module before starting an Open vSwitch or Linuxbridge agent using + iptables based firewall. Otherwise the agent will fail to start.