Merge pull request #697 from pitkley/support-ctc-same-network-verdict

Support default network verdict within the same network

.github/workflows/it.yml

@@ -69,18 +69,6 @@ jobs:
           runs-on: ubuntu-20.04
         - dind-image: docker:18.06-dind
           runs-on: ubuntu-20.04
-        - dind-image: docker:18.03-dind
-          runs-on: ubuntu-20.04
-        - dind-image: docker:17.12-dind
-          runs-on: ubuntu-20.04
-        - dind-image: docker:17.09-dind
-          runs-on: ubuntu-20.04
-        - dind-image: docker:17.07-dind
-          runs-on: ubuntu-20.04
-        - dind-image: docker:17.06-dind
-          runs-on: ubuntu-20.04
-        - dind-image: docker:1.13-dind
-          runs-on: ubuntu-20.04
 
     services:
       dind:

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ## Unreleased
 
+* Add [`same_network_verdict` option](https://dfw.rs/latest/dfw/types/struct.ContainerToContainer.html#structfield.same_network_verdict) to container-to-container configuration, enabling users to specify whether traffic between containers within the same network should be allowed or not.
 * Replace library used to communicate with Docker (which also fixes [#411]).
 
     This release replaces the previously used library [shiplift] by [bollard].

README.md

@@ -339,14 +339,8 @@ DFW is continuously and automatically tested with the following stable Docker ve
 * `19.03`
 * `18.09`
 * `18.06`
-* `18.03`
-* `17.12`
-* `17.09`
-* `17.07`
-* `17.06`
-* `1.13`
-
-Docker version 20.10 is also officially supported by DFW, although it is not yet automatically tested.
+
+Docker versions between 18.03 and 1.13 should also work, but are not automatically tested anymore due to lacking Docker BuildKit support.
 
 ## <a name="versionbumppolicy"></a> Version bump policy
 

resources/test/docker/ctc-network-policies/conf.toml

@@ -0,0 +1,7 @@
+[container_to_container]
+default_policy = "drop"
+same_network_verdict = "accept"
+
+[[container_to_container.rules]]
+network = "PROJECT_default"
+verdict = "reject"

resources/test/docker/ctc-network-policies/docker-compose.yml

@@ -0,0 +1,12 @@
+version: '2'
+
+services:
+  a:
+    image: nginx:alpine
+    networks:
+    - default
+    - other
+
+networks:
+  default:
+  other:

resources/test/docker/ctc-network-policies/iptables/expected-iptables-v4.txt

@@ -0,0 +1,32 @@
+*filter
+:DFWRS_FORWARD - [0:0]
+:DFWRS_INPUT - [0:0]
+:FORWARD - [0:0]
+:INPUT - [0:0]
+-F DFWRS_FORWARD
+-A DFWRS_FORWARD -m state --state INVALID -j DROP
+-A DFWRS_FORWARD -m state --state RELATED,ESTABLISHED -j ACCEPT
+-A DFWRS_FORWARD -i $input=bridge -o $output=bridge -j REJECT	"$input" == "$output"
+-A DFWRS_FORWARD -i $input=bridge -o $output=bridge -j ACCEPT	"$input" == "$output"
+-A DFWRS_FORWARD -i $input=bridge -o $output=bridge -j ACCEPT	"$input" == "$output"
+-A DFWRS_FORWARD -i $input=bridge -o $output=bridge -j ACCEPT	"$input" == "$output"
+-A DFWRS_FORWARD -i $input=bridge -o $output=bridge -j ACCEPT	"$input" == "$output"
+-A DFWRS_FORWARD -i $input=bridge -o $output=bridge -j ACCEPT	"$input" == "$output"
+-A DFWRS_FORWARD -j DROP
+-F DFWRS_INPUT
+-A DFWRS_INPUT -m state --state INVALID -j DROP
+-A DFWRS_INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT
+-A DFWRS_INPUT -i docker0 -j ACCEPT
+-A FORWARD -j DFWRS_FORWARD
+-A INPUT -j DFWRS_INPUT
+COMMIT
+*nat
+:DFWRS_POSTROUTING - [0:0]
+:DFWRS_PREROUTING - [0:0]
+:POSTROUTING - [0:0]
+:PREROUTING - [0:0]
+-F DFWRS_POSTROUTING
+-F DFWRS_PREROUTING
+-A POSTROUTING -j DFWRS_POSTROUTING
+-A PREROUTING -j DFWRS_PREROUTING
+COMMIT

resources/test/docker/ctc-network-policies/iptables/expected-iptables-v6.txt

@@ -0,0 +1,16 @@
+*filter
+:DFWRS_FORWARD - [0:0]
+:DFWRS_INPUT - [0:0]
+-F DFWRS_FORWARD
+-A DFWRS_FORWARD -m state --state INVALID -j DROP
+-A DFWRS_FORWARD -m state --state RELATED,ESTABLISHED -j ACCEPT
+-F DFWRS_INPUT
+-A DFWRS_INPUT -m state --state INVALID -j DROP
+-A DFWRS_INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT
+COMMIT
+*nat
+:DFWRS_POSTROUTING - [0:0]
+:DFWRS_PREROUTING - [0:0]
+-F DFWRS_POSTROUTING
+-F DFWRS_PREROUTING
+COMMIT

resources/test/docker/ctc-network-policies/nftables/expected-nftables.txt

@@ -0,0 +1,24 @@
+add table inet dfw
+flush table inet dfw
+add chain inet dfw input { type filter hook input priority -5 ; }
+add rule inet dfw input ct state invalid drop
+add rule inet dfw input ct state { related, established } accept
+add chain inet dfw forward { type filter hook forward priority -5 ; }
+add rule inet dfw forward ct state invalid drop
+add rule inet dfw forward ct state { related, established } accept
+add table ip dfw
+flush table ip dfw
+add chain ip dfw prerouting { type nat hook prerouting priority -105 ; }
+add chain ip dfw postrouting { type nat hook postrouting priority 95 ; }
+add table ip6 dfw
+flush table ip6 dfw
+add chain ip6 dfw prerouting { type nat hook prerouting priority -105 ; }
+add chain ip6 dfw postrouting { type nat hook postrouting priority 95 ; }
+add rule inet dfw input meta iifname docker0 meta mark set 0xdf accept
+add chain inet dfw forward { policy drop ; }
+add rule inet dfw forward meta iifname $input=bridge oifname $output=bridge meta mark set 0xdf reject	"$input" == "$output"
+add rule inet dfw forward meta iifname $input=bridge oifname $output=bridge meta mark set 0xdf accept	"$input" == "$output"
+add rule inet dfw forward meta iifname $input=bridge oifname $output=bridge meta mark set 0xdf accept	"$input" == "$output"
+add rule inet dfw forward meta iifname $input=bridge oifname $output=bridge meta mark set 0xdf accept	"$input" == "$output"
+add rule inet dfw forward meta iifname $input=bridge oifname $output=bridge meta mark set 0xdf accept	"$input" == "$output"
+add rule inet dfw forward meta iifname $input=bridge oifname $output=bridge meta mark set 0xdf accept	"$input" == "$output"

src/iptables/process.rs

@@ -252,6 +252,30 @@ impl Process<Iptables> for ContainerToContainer {
             rules.append(&mut ctc_rules);
         }
 
+        if let Some(same_network_verdict) = self.same_network_verdict {
+            for network in ctx.network_map.values() {
+                let network_id = network.id.as_ref().expect("Docker network ID missing");
+                let bridge_name = get_bridge_name(network_id)?;
+                trace!(ctx.logger, "Got bridge name";
+                       o!("network_name" => &network.name,
+                          "bridge_name" => &bridge_name));
+
+                let ipt_rule = Rule::new("filter", DFW_FORWARD_CHAIN)
+                    .in_interface(&bridge_name)
+                    .out_interface(&bridge_name)
+                    .jump(&same_network_verdict.to_string().to_uppercase())
+                    .build()?;
+
+                debug!(ctx.logger, "Add forward rule for same network verdict for bridge";
+                       o!("part" => "container_to_container",
+                          "bridge_name" => bridge_name,
+                          "same_network_verdict" => same_network_verdict,
+                          "rule" => &ipt_rule));
+
+                rules.push(append_built_rule(IptablesRuleDiscriminants::V4, &ipt_rule));
+            }
+        }
+
         // Enforce default policy for container-to-container communication.
         rules.push(append_rule(
             IptablesRuleDiscriminants::V4,

src/nftables/process.rs

@@ -330,6 +330,30 @@ impl Process<Nftables> for ContainerToContainer {
             rules.append(&mut ctc_rules);
         }
 
+        if let Some(same_network_verdict) = self.same_network_verdict {
+            for network in ctx.network_map.values() {
+                let network_id = network.id.as_ref().expect("Docker network ID missing");
+                let bridge_name = get_bridge_name(network_id)?;
+                trace!(ctx.logger, "Got bridge name";
+                       o!("network_name" => &network.name,
+                          "bridge_name" => &bridge_name));
+
+                let rule = RuleBuilder::default()
+                    .in_interface(&bridge_name)
+                    .out_interface(&bridge_name)
+                    .verdict(same_network_verdict)
+                    .build()?;
+
+                debug!(ctx.logger, "Add forward rule for same network verdict for bridge";
+                       o!("part" => "container_to_container",
+                          "bridge_name" => bridge_name,
+                          "same_network_verdict" => same_network_verdict,
+                          "rule" => &rule));
+
+                rules.push(add_rule(Family::Inet, "dfw", "forward", &rule));
+            }
+        }
+
         Ok(Some(rules))
     }
 }

src/process.rs

@@ -164,7 +164,7 @@ where
             .cloned();
         let primary_external_network_interface = external_network_interfaces
             .as_ref()
-            .and_then(|v| v.get(0))
+            .and_then(|v| v.first())
             .map(|s| s.to_owned());
 
         Ok(ProcessContext {

src/types.rs

@@ -192,6 +192,69 @@ pub struct ContainerToContainer {
     ///
     /// To permanently set this configuration, take a look at `man sysctl.d` and `man sysctl.conf`.
     pub default_policy: ChainPolicy,
+    /// Configure whether traffic between containers within the same network should be allowed or
+    /// not.
+    ///
+    /// This option is more specific than [`default_policy`], applying to traffic between containers
+    /// on the same network, rather than also applying across networks. This option has precedence
+    /// over the [`default_policy`] for traffic on the same network.
+    ///
+    /// ## Example
+    ///
+    /// Setting the `same_network_verdict` to `accept` will allow all traffic between containers on
+    /// the same network to pass, regardless of the [`default_policy`] configured:
+    ///
+    /// ```
+    /// # use dfw::nftables::Nftables;
+    /// # use dfw::types::*;
+    /// # use toml;
+    /// # toml::from_str::<DFW<Nftables>>(r#"
+    /// [container_to_container]
+    /// default_policy = "drop"
+    /// same_network_verdict = "accept"
+    /// # "#).unwrap();
+    /// ```
+    ///
+    /// If you want to allow traffic between containers on the same networks in general, but want to
+    /// restrict traffic on some networks, you can additionally add [container-to-container rules]
+    /// to disallow traffic between containers for the desired networks:
+    ///
+    /// ```
+    /// # use dfw::nftables::Nftables;
+    /// # use dfw::types::*;
+    /// # use toml;
+    /// # toml::from_str::<DFW<Nftables>>(r#"
+    /// [container_to_container]
+    /// default_policy = "drop"
+    /// same_network_verdict = "accept"
+    ///
+    /// [[container_to_container.rules]]
+    /// network = "restricted_network"
+    /// verdict = "reject"
+    /// # "#).unwrap();
+    /// ```
+    ///
+    /// [container-to-container rules]: struct.ContainerToContainerRule.html
+    ///
+    /// ## Host configuration
+    ///
+    /// Depending on how your host is configured, traffic whose origin and destination interface are
+    /// the same bridge (network) is _not_ filtered by the kernel netfilter module.
+    ///
+    /// This means that this verdict is only honored if your kernel has the `br_netfilter`
+    /// kernel-module available and the sysctl `net.bridge.bridge-nf-call-iptables` is set to `1`.
+    /// Otherwise traffic between containers on the same network will always be allowed.
+    ///
+    /// You can set the sysctl-value temporarily like this:
+    ///
+    /// ```text
+    /// sysctl net.bridge.bridge-nf-call-iptables=1
+    /// ```
+    ///
+    /// To permanently set this configuration, take a look at `man sysctl.d` and `man sysctl.conf`.
+    ///
+    /// [`default_policy`]: #structfield.default_policy
+    pub same_network_verdict: Option<RuleVerdict>,
     /// An optional list of rules, see
     /// [`ContainerToContainerRule`](struct.ContainerToContainerRule.html).
     ///

tests/dfw.rs

@@ -399,6 +399,7 @@ dfw_tests!(
     "05";
     "06";
     "07";
+    "ctc-network-policies";
 
     R F "001_gh_166_01" "001-gh-166/01";
     R F "001_gh_166_02" "001-gh-166/02";

tests/types.rs

@@ -58,6 +58,7 @@ fn parse_conf_file() {
     };
     let container_to_container = ContainerToContainer {
         default_policy: ChainPolicy::Drop,
+        same_network_verdict: None,
         rules: Some(vec![ContainerToContainerRule {
             network: "network".to_owned(),
             src_container: Some("src_container".to_owned()),
@@ -161,6 +162,7 @@ fn parse_conf_path() {
     };
     let container_to_container = ContainerToContainer {
         default_policy: ChainPolicy::Drop,
+        same_network_verdict: None,
         rules: Some(vec![ContainerToContainerRule {
             network: "network".to_owned(),
             src_container: Some("src_container".to_owned()),
@@ -594,7 +596,7 @@ fn ensure_backwards_compatibility_v1() {
             source_cidr_v4,
             source_cidr_v6,
             ..
-        } = rules.get(0).unwrap();
+        } = rules.first().unwrap();
         assert!(source_cidr_v4.is_some());
         assert!(source_cidr_v6.is_none());
     }