add NETAVARK_ISOLATION_3 chain for strict isolation

Signed-off-by: yassi-github <[email protected]>

src/firewall/iptables.rs

@@ -66,7 +66,7 @@ impl firewall::FirewallDriver for IptablesDriver {
                     &network_setup.network_hash_name,
                     is_ipv6,
                     interface.to_string(),
-                    network_setup.isolation,
+                    &network_setup.isolation,
                 );
 
                 create_network_chains(chains)?;
@@ -105,7 +105,7 @@ impl firewall::FirewallDriver for IptablesDriver {
                     &tear.config.network_hash_name,
                     is_ipv6,
                     interface.to_string(),
-                    tear.config.isolation,
+                    &tear.config.isolation,
                 );
 
                 for c in &chains {

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::{PortForwardConfig, IsolateOption};
 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: &'a IsolateOption,
 ) -> Vec<VarkChain<'a>> {
     let mut chains = Vec::new();
     let prefixed_network_hash_name = format!("{}-{}", "NETAVARK", network_hash_name);
@@ -238,7 +239,19 @@ 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;
+
+    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 +262,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),
@@ -286,7 +296,40 @@ pub fn get_network_chains<'a>(
 
         // PUSH CHAIN
         chains.push(netavark_isolation_chain_1);
-        chains.push(netavark_isolation_chain_2)
+    }
+
+    // ensure NETAVARK_ISOLATION_2 exists.
+    chains.push(netavark_isolation_chain_2);
+
+    // create DROP rule for non-isolations to enforce strict isolation rules.
+    if let IsolateOption::Never = isolation {
+        // NETAVARK_ISOLATION_3
+        let mut netavark_isolation_chain_3 = VarkChain::new(
+            conn,
+            FILTER.to_string(),
+            NETAVARK_ISOLATION_3.to_string(),
+            None,
+        );
+        netavark_isolation_chain_3.create = true;
+
+        // 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),
+        });
+        chains.push(netavark_isolation_chain_3);
     }
 
     forward_chain.build_rule(VarkRule {

src/network/bridge.rs

@@ -17,13 +17,15 @@ use crate::{
 
 use super::{
     constants::{
-        NO_CONTAINER_INTERFACE_ERROR, OPTION_ISOLATE, OPTION_METRIC, OPTION_MTU,
-        OPTION_NO_DEFAULT_ROUTE,
+        NO_CONTAINER_INTERFACE_ERROR, OPTION_ISOLATE,
+        ISOLATE_OPTION_TRUE, ISOLATE_OPTION_FALSE, ISOLATE_OPTION_STRICT,
+        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,
+        IsolateOption,
     },
     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,9 @@ 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: String =
+        //     parse_option(&self.info.network.options, OPTION_ISOLATE)?.unwrap_or(ISOLATE_OPTION_FALSE.to_string());
+        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,14 +306,14 @@ impl<'a> Bridge<'a> {
         &'a self,
         container_addresses: &Vec<IpNet>,
         nameservers: &'a Vec<IpAddr>,
-        isolate: bool,
+        isolate: &'a IsolateOption,
     ) -> NetavarkResult<(SetupNetwork, PortForwardConfig)> {
         let id_network_hash =
             CoreUtils::create_network_hash(&self.info.network.name, MAX_HASH_SIZE);
         let sn = SetupNetwork {
             net: self.info.network.clone(),
             network_hash_name: id_network_hash.clone(),
-            isolation: isolate,
+            isolation: isolate.clone(),
         };
 
         let mut has_ipv4 = false;
@@ -359,7 +362,7 @@ impl<'a> Bridge<'a> {
         let (sn, spf) = self.get_firewall_conf(
             &data.ipam.container_addresses,
             &data.ipam.nameservers,
-            data.isolate,
+            &data.isolate,
         )?;
 
         self.info.firewall.setup_network(sn)?;
@@ -384,20 +387,12 @@ impl<'a> Bridge<'a> {
     fn teardown_firewall(&self, complete_teardown: bool) -> NetavarkResult<()> {
         // we have to allocate the vecoros here in the top level to avoid
         // "borrow later used" problems
-        let (container_addresses, nameservers);
+        let (container_addresses, nameservers, isolate);
 
-        let (container_addresses_ref, nameservers_ref, isolate) = match &self.data {
-            Some(d) => (&d.ipam.container_addresses, &d.ipam.nameservers, d.isolate),
+        let (container_addresses_ref, nameservers_ref, isolate_ref) = 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
-                    }
-                };
+                isolate = get_isolate_option(&self.info.network.options);
 
                 (container_addresses, nameservers) =
                     match get_ipam_addresses(self.info.per_network_opts, self.info.network) {
@@ -408,12 +403,12 @@ impl<'a> Bridge<'a> {
                             (Vec::new(), Vec::new())
                         }
                     };
-                (&container_addresses, &nameservers, isolate)
+                (&container_addresses, &nameservers, &isolate)
             }
         };
 
         let (sn, spf) =
-            self.get_firewall_conf(container_addresses_ref, nameservers_ref, isolate)?;
+            self.get_firewall_conf(container_addresses_ref, nameservers_ref, isolate_ref)?;
 
         let tn = TearDownNetwork {
             config: sn,
@@ -706,3 +701,22 @@ fn remove_link(
     }
     Ok(false)
 }
+
+fn get_isolate_option(
+    opts: &Option<HashMap<String, String>>,
+) -> IsolateOption {
+    let isolate = match parse_option(opts, OPTION_ISOLATE) {
+        Ok(i) => i.unwrap_or(ISOLATE_OPTION_FALSE.to_string()),
+        Err(e) => {
+            // just log we still try to do as much as possible for cleanup
+            error!("failed to parse {} option: {}", OPTION_ISOLATE, e);
+            ISOLATE_OPTION_FALSE.to_string()
+        }
+    };
+    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, Debug, PartialEq)]
+pub enum IsolateOption {
+    Strict,
+    Nomal,
+    Never,
+}