Merge pull request #833 from projectcalico/smc-ensure-rp-filter

Set the kernel's default rp_filter setting for new interfaces to strict.

CHANGES.md

@@ -7,6 +7,8 @@
   (which contains additional patches) is still required.
 - Reduce occupancy of Felix's tag resolution index in the common case
   where IP addresses only have a single owner.
+- Felix now sets the default.rp_filter sysctl to ensure that endpoints
+  come up with the Kernel's RPF check enabled by default.
 
 ## 1.1.0
 

calico/felix/devices.py

@@ -33,18 +33,34 @@
 _log = logging.getLogger(__name__)
 
 
-def check_kernel_config():
+def configure_global_kernel_config():
     """
-    Checks the kernel configuration for problems that would break our
-    security guarantees, for example.
+    Configures the global kernel config.  In particular, sets the flags
+    that we rely on to ensure security, such as the kernel's RPF check.
 
     :raises BadKernelConfig if a problem is detected.
     """
 
-    # To prevent workloads from spoofing their IP addresses, we need to set the
-    # per-interface rp_filter setting to 1, which enables strict RPF checking.
-    # Verify that the kernel's global setting won't override our per-interface
-    # setting with an insecure value.
+    # For IPv4, we rely on the kernel's reverse path filtering to prevent
+    # workloads from spoofing their IP addresses.
+    #
+    # The RPF check for a particular interface is controlled by several
+    # sysctls:
+    #
+    # - ipv4.conf.all.rp_filter is a global override
+    # - ipv4.conf.default.rp_filter controls the value that is set on a newly
+    #   created interface
+    # - ipv4.conf.<interface>.rp_filter controls a particular interface.
+    #
+    # The algorithm for combining the global override and per-interface values
+    # is to take the *numeric* maximum between the two.  The values are:
+    # 0=off, 1=strict, 2=loose.  "loose" is not suitable for Calico since it
+    # would allow workloads to spoof packets from other workloads on the same
+    # host.  Hence, we need the global override to be <=1 or it would override
+    # the per-interface setting to "strict" that we require.
+    #
+    # We bail out rather than simply setting it because setting 2, "loose",
+    # is unusual and it is likely to have been set deliberately.
     ps_name = "/proc/sys/net/ipv4/conf/all/rp_filter"
     rp_filter = int(_read_proc_sys(ps_name))
     if rp_filter > 1:
@@ -54,6 +70,11 @@ def check_kernel_config():
                       "0 or 1.")
         raise BadKernelConfig("net.ipv4.conf.all.rp_filter set to 'loose'")
 
+    # Make sure the default for new interfaces is set to strict checking so
+    # that there's no race when a new interface is added and felix hasn't
+    # configured it yet.
+    _write_proc_sys("/proc/sys/net/ipv4/conf/default/rp_filter", "1")
+
 
 def interface_exists(interface):
     """

calico/felix/felix.py

@@ -66,8 +66,9 @@ def _main_greenlet(config):
         config_loaded = etcd_api.load_config(async=False)
         config_loaded.wait()
 
-        # Check for any incorrect kernel configuration that would break Calico.
-        devices.check_kernel_config()
+        # Ensure the Kernel's global options are correctly configured for
+        # Calico.
+        devices.configure_global_kernel_config()
 
         _log.info("Main greenlet: Configuration loaded, starting remaining "
                   "actors...")

calico/felix/test/test_devices.py

@@ -48,16 +48,21 @@ def setUp(self):
     def tearDown(self):
         pass
 
-    def test_check_kernel_config(self):
+    def test_configure_global_kernel_config(self):
         with mock.patch("calico.felix.devices._read_proc_sys",
                         autospec=True, return_value="1") as m_read_proc_sys:
-            devices.check_kernel_config()
+            with mock.patch("calico.felix.devices._write_proc_sys",
+                            autospec=True) as m_write_proc_sys:
+                devices.configure_global_kernel_config()
+        m_write_proc_sys.assert_called_once_with(
+            "/proc/sys/net/ipv4/conf/default/rp_filter", "1"
+        )
 
-    def test_check_kernel_config_bad_rp_filter(self):
+    def test_configure_global_kernel_config_bad_rp_filter(self):
         with mock.patch("calico.felix.devices._read_proc_sys",
                         autospec=True, return_value="2") as m_read_proc_sys:
             self.assertRaises(devices.BadKernelConfig,
-                              devices.check_kernel_config)
+                              devices.configure_global_kernel_config)
 
     def test_read_proc_sys(self):
         m_open = mock.mock_open(read_data="1\n")

calico/felix/test/test_felix.py

@@ -48,7 +48,8 @@ def tearDown(self):
         else:
             sys.modules['etcd'] = self._real_etcd
 
-    @mock.patch("calico.felix.devices.check_kernel_config", autospec=True)
+    @mock.patch("calico.felix.devices.configure_global_kernel_config",
+                autospec=True)
     @mock.patch("calico.felix.devices.interface_up",
                 return_value=False, autospec=True)
     @mock.patch("calico.felix.devices.interface_exists",
@@ -65,7 +66,7 @@ def test_main_greenlet(self, m_iwait, m_MasqueradeManager,
                            m_IptablesUpdater, m_UpdateSplitter,
                            m_start, m_load,
                            m_ipset_4, m_check_call, m_iface_exists,
-                           m_iface_up, m_check_kernel_config):
+                           m_iface_up, m_configure_global_kernel_config):
         m_IptablesUpdater.return_value.greenlet = mock.Mock()
         m_MasqueradeManager.return_value.greenlet = mock.Mock()
         m_UpdateSplitter.return_value.greenlet = mock.Mock()
@@ -84,7 +85,7 @@ def test_main_greenlet(self, m_iwait, m_MasqueradeManager,
         m_load.assert_called_once_with(async=False)
         m_iface_exists.assert_called_once_with("tunl0")
         m_iface_up.assert_called_once_with("tunl0")
-        m_check_kernel_config.assert_called_once_with()
+        m_configure_global_kernel_config.assert_called_once_with()
 
         # Check all IptablesUpdaters get passed to the splitter, which handles
         # cleanup.