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.

Changes made to this backport:
   neutron/agent/linux/iptables_firewall.py
       - removed deprecation warning when setting sysctl values to 1 as
         they are planned to be removed in Ocata
   neutron/cmd/sanity/checks.py
       - Re-implemented the flow to check only for presence of sysctl
         options instead of checking the values. Kernel options are set
         in runtime thus the values don't matter.

Depends-On: Id6bfd9595f0772a63d1096ef83ebbb6cd630fafd
Change-Id: I9137ea017624ac92a05f73863b77f9ee4681bbe7
Related-Bug: #1622914
(cherry picked from commit e83a44b96a)

etc/neutron/rootwrap.d/iptables-firewall.filters

@@ -8,20 +8,19 @@
 
 [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

neutron/agent/linux/iptables_firewall.py

@@ -23,7 +23,7 @@ from oslo_log import log as logging
 from oslo_utils import netutils
 import six
 
-from neutron._i18n import _LI
+from neutron._i18n import _, _LI
 from neutron.agent import firewall
 from neutron.agent.linux import ip_conntrack
 from neutron.agent.linux import ipset_manager
@@ -109,15 +109,17 @@ 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':
+                utils.execute(
+                    ['sysctl', '-w', '%s=1' % knob], run_as_root=True)
 
     @property
     def ports(self):

neutron/cmd/sanity/checks.py

@@ -228,6 +228,17 @@ def dhcp_release6_supported():
     return True
 
 
+def bridge_firewalling_enabled():
+    cmd = ['sysctl', '-N', 'net.bridge']
+    entries = agent_utils.execute(cmd, run_as_root=True)
+    for proto in ('arp', 'ip', 'ip6'):
+        knob = 'net.bridge.bridge-nf-call-%stables' % proto
+        if knob not in entries:
+            LOG.debug("sysctl value %s not present on this system.", knob)
+            return False
+    return True
+
+
 class KeepalivedIPv6Test(object):
     def __init__(self, ha_port, gw_port, gw_vip, default_gw):
         self.ha_port = ha_port

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

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

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

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

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.