Merge pull request #703 from yassi-github/strict-isolation

feature: isolation among all netavark bridge

src/firewall/varktables/types.rs

@@ -3,7 +3,7 @@ use crate::firewall::varktables::helpers::{
     add_chain_unique, append_unique, remove_if_rule_exists,
 };
 use crate::firewall::varktables::types::TeardownPolicy::{Never, OnComplete};
-use crate::network::internal_types::PortForwardConfig;
+use crate::network::internal_types::{IsolateOption, PortForwardConfig};
 use ipnet::IpNet;
 use iptables::IPTables;
 use log::debug;
@@ -26,6 +26,7 @@ const MARK: &str = "MARK";
 const DNAT: &str = "DNAT";
 const NETAVARK_ISOLATION_1: &str = "NETAVARK_ISOLATION_1";
 const NETAVARK_ISOLATION_2: &str = "NETAVARK_ISOLATION_2";
+const NETAVARK_ISOLATION_3: &str = "NETAVARK_ISOLATION_3";
 
 const CONTAINER_DN_CHAIN: &str = "NETAVARK-DN-";
 
@@ -194,7 +195,7 @@ pub fn get_network_chains<'a>(
     network_hash_name: &'a str,
     is_ipv6: bool,
     interface_name: String,
-    isolation: bool,
+    isolation: IsolateOption,
 ) -> Vec<VarkChain<'a>> {
     let mut chains = Vec::new();
     let prefixed_network_hash_name = format!("{}-{}", "NETAVARK", network_hash_name);
@@ -238,7 +239,31 @@ pub fn get_network_chains<'a>(
     // used to prepend specific rules
     let mut ind = 1;
 
-    if isolation {
+    // NETAVARK_ISOLATION_2
+    // NETAVARK_ISOLATION_2 chain must always exist,
+    // because non-isolation creates DROP rule in NETAVARK_ISOLATION_3
+    // and NETAVARK_ISOLATION_3 references this as a jump target.
+    let mut netavark_isolation_chain_2 = VarkChain::new(
+        conn,
+        FILTER.to_string(),
+        NETAVARK_ISOLATION_2.to_string(),
+        None,
+    );
+    netavark_isolation_chain_2.create = true;
+
+    // NETAVARK_ISOLATION_3
+    // NETAVARK_ISOLATION_3 chain must exist when IsolateOption is Never or Strict.
+    // bacause non-isolation creates DROP rule in NETAVARK_ISOLATION_3.
+    // and strict isolation references NETAVARK_ISOLATION_3 as a jump target.
+    let mut netavark_isolation_chain_3 = VarkChain::new(
+        conn,
+        FILTER.to_string(),
+        NETAVARK_ISOLATION_3.to_string(),
+        None,
+    );
+    netavark_isolation_chain_3.create = true;
+
+    if let IsolateOption::Nomal | IsolateOption::Strict = isolation {
         debug!("Add extra isolate rules");
         // NETAVARK_ISOLATION_1
         let mut netavark_isolation_chain_1 = VarkChain::new(
@@ -249,27 +274,24 @@ pub fn get_network_chains<'a>(
         );
         netavark_isolation_chain_1.create = true;
 
-        // NETAVARK_ISOLATION_2
-        let mut netavark_isolation_chain_2 = VarkChain::new(
-            conn,
-            FILTER.to_string(),
-            NETAVARK_ISOLATION_2.to_string(),
-            None,
-        );
-        netavark_isolation_chain_2.create = true;
-
         // -A FORWARD -j NETAVARK_ISOLATION_1
         forward_chain.build_rule(VarkRule {
             rule: format!("-j {}", NETAVARK_ISOLATION_1),
             position: Some(ind),
             td_policy: Some(TeardownPolicy::OnComplete),
         });
 
-        // NETAVARK_ISOLATION_1 -i bridge_name ! -o bridge_name -j DROP
+        let netavark_isolation_1_target = if let IsolateOption::Strict = isolation {
+            // NETAVARK_ISOLATION_1 -i bridge_name ! -o bridge_name -j NETAVARK_ISOLATION_3
+            NETAVARK_ISOLATION_3
+        } else {
+            // NETAVARK_ISOLATION_1 -i bridge_name ! -o bridge_name -j NETAVARK_ISOLATION_2
+            NETAVARK_ISOLATION_2
+        };
         netavark_isolation_chain_1.build_rule(VarkRule {
             rule: format!(
                 "-i {} ! -o {} -j {}",
-                interface_name, interface_name, NETAVARK_ISOLATION_2
+                interface_name, interface_name, netavark_isolation_1_target
             ),
             position: Some(ind),
             td_policy: Some(TeardownPolicy::OnComplete),
@@ -282,13 +304,40 @@ pub fn get_network_chains<'a>(
             td_policy: Some(TeardownPolicy::OnComplete),
         });
 
+        // NETAVARK_ISOLATION_3 -j NETAVARK_ISOLATION_2
+        netavark_isolation_chain_3.build_rule(VarkRule {
+            rule: format!("-j {}", NETAVARK_ISOLATION_2),
+            position: Some(ind),
+            td_policy: Some(TeardownPolicy::Never),
+        });
+
         ind += 1;
 
         // PUSH CHAIN
         chains.push(netavark_isolation_chain_1);
-        chains.push(netavark_isolation_chain_2)
+    } else {
+        // create DROP rule for non-isolations to enforce strict isolation rules.
+
+        // NETAVARK_ISOLATION_3 -o bridge_name -j DROP
+        netavark_isolation_chain_3.build_rule(VarkRule {
+            rule: format!("-o {} -j {}", interface_name, "DROP"),
+            position: Some(ind),
+            td_policy: Some(TeardownPolicy::OnComplete),
+        });
+
+        // NETAVARK_ISOLATION_3 -j NETAVARK_ISOLATION_2
+        netavark_isolation_chain_3.build_rule(VarkRule {
+            rule: format!("-j {}", NETAVARK_ISOLATION_2),
+            // position +1 to place this rule under all of NETAVARK_ISOLATION_3 DROP rules.
+            position: Some(ind + 1),
+            td_policy: Some(TeardownPolicy::Never),
+        });
     }
 
+    // PUSH CHAIN
+    chains.push(netavark_isolation_chain_2);
+    chains.push(netavark_isolation_chain_3);
+
     forward_chain.build_rule(VarkRule {
         rule: format!(
             "-m comment --comment 'netavark firewall plugin rules' -j {}",

src/network/bridge.rs

@@ -17,13 +17,15 @@ use crate::{
 
 use super::{
     constants::{
+        ISOLATE_OPTION_FALSE, ISOLATE_OPTION_STRICT, ISOLATE_OPTION_TRUE,
         NO_CONTAINER_INTERFACE_ERROR, OPTION_ISOLATE, OPTION_METRIC, OPTION_MTU,
         OPTION_NO_DEFAULT_ROUTE,
     },
     core_utils::{self, get_ipam_addresses, join_netns, parse_option, CoreUtils},
     driver::{self, DriverInfo},
     internal_types::{
-        IPAMAddresses, PortForwardConfig, SetupNetwork, TearDownNetwork, TeardownPortForward,
+        IPAMAddresses, IsolateOption, PortForwardConfig, SetupNetwork, TearDownNetwork,
+        TeardownPortForward,
     },
     netlink,
     types::StatusBlock,
@@ -43,7 +45,7 @@ struct InternalData {
     /// mtu for the network interfaces (0 if default)
     mtu: u32,
     /// if this network should be isolated from others
-    isolate: bool,
+    isolate: IsolateOption,
     /// Route metric for any default routes added for the network
     metric: Option<u32>,
     /// if set, no default gateway will be added
@@ -75,8 +77,7 @@ impl driver::NetworkDriver for Bridge<'_> {
         let ipam = get_ipam_addresses(self.info.per_network_opts, self.info.network)?;
 
         let mtu: u32 = parse_option(&self.info.network.options, OPTION_MTU)?.unwrap_or(0);
-        let isolate: bool =
-            parse_option(&self.info.network.options, OPTION_ISOLATE)?.unwrap_or(false);
+        let isolate: IsolateOption = get_isolate_option(&self.info.network.options)?;
         let metric: u32 = parse_option(&self.info.network.options, OPTION_METRIC)?.unwrap_or(100);
         let no_default_route: bool =
             parse_option(&self.info.network.options, OPTION_NO_DEFAULT_ROUTE)?.unwrap_or(false);
@@ -303,7 +304,7 @@ impl<'a> Bridge<'a> {
         &'a self,
         container_addresses: &Vec<IpNet>,
         nameservers: &'a Vec<IpAddr>,
-        isolate: bool,
+        isolate: IsolateOption,
     ) -> NetavarkResult<(SetupNetwork, PortForwardConfig)> {
         let id_network_hash =
             CoreUtils::create_network_hash(&self.info.network.name, MAX_HASH_SIZE);
@@ -389,15 +390,11 @@ impl<'a> Bridge<'a> {
         let (container_addresses_ref, nameservers_ref, isolate) = match &self.data {
             Some(d) => (&d.ipam.container_addresses, &d.ipam.nameservers, d.isolate),
             None => {
-                // options are not yet parsed
-                let isolate: bool = match parse_option(&self.info.network.options, OPTION_ISOLATE) {
-                    Ok(i) => i.unwrap_or(false),
-                    Err(e) => {
-                        // just log we still try to do as much as possible for cleanup
-                        error!("failed to parse {} option: {}", OPTION_ISOLATE, e);
-                        false
-                    }
-                };
+                let isolate = get_isolate_option(&self.info.network.options).unwrap_or_else(|e| {
+                    // just log we still try to do as much as possible for cleanup
+                    error!("failed to parse {} option: {}", OPTION_ISOLATE, e);
+                    IsolateOption::Never
+                });
 
                 (container_addresses, nameservers) =
                     match get_ipam_addresses(self.info.per_network_opts, self.info.network) {
@@ -706,3 +703,14 @@ fn remove_link(
     }
     Ok(false)
 }
+
+fn get_isolate_option(opts: &Option<HashMap<String, String>>) -> NetavarkResult<IsolateOption> {
+    let isolate = parse_option(opts, OPTION_ISOLATE)?.unwrap_or(ISOLATE_OPTION_FALSE.to_string());
+    // return isolate option value "false" if unknown value or no value passed
+    Ok(match isolate.as_str() {
+        ISOLATE_OPTION_STRICT => IsolateOption::Strict,
+        ISOLATE_OPTION_TRUE => IsolateOption::Nomal,
+        ISOLATE_OPTION_FALSE => IsolateOption::Never,
+        _ => IsolateOption::Never,
+    })
+}

src/network/constants.rs

@@ -13,6 +13,9 @@ pub const DRIVER_IPVLAN: &str = "ipvlan";
 pub const DRIVER_MACVLAN: &str = "macvlan";
 
 pub const OPTION_ISOLATE: &str = "isolate";
+pub const ISOLATE_OPTION_TRUE: &str = "true";
+pub const ISOLATE_OPTION_FALSE: &str = "false";
+pub const ISOLATE_OPTION_STRICT: &str = "strict";
 pub const OPTION_MTU: &str = "mtu";
 pub const OPTION_MODE: &str = "mode";
 pub const OPTION_METRIC: &str = "metric";

src/network/internal_types.rs

@@ -18,7 +18,7 @@ pub struct SetupNetwork {
     /// hash id for the network
     pub network_hash_name: String,
     /// isolation determines whether the network can communicate with others outside of its interface
-    pub isolation: bool,
+    pub isolation: IsolateOption,
 }
 
 #[derive(Debug)]
@@ -73,3 +73,11 @@ pub struct IPAMAddresses {
     pub net_addresses: Vec<types::NetAddress>,
     pub nameservers: Vec<IpAddr>,
 }
+
+// IsolateOption is used to select isolate option value
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub enum IsolateOption {
+    Strict,
+    Nomal,
+    Never,
+}