From b052e2f5e560f4baf4ff8a5e524a71fc92b8946d Mon Sep 17 00:00:00 2001 From: Pit Kleyersburg Date: Wed, 13 Sep 2023 18:45:26 +0200 Subject: [PATCH 1/3] Support setting a default verdict for traffic within container networks --- CHANGELOG.md | 1 + .../docker/ctc-network-policies/conf.toml | 7 +++ .../ctc-network-policies/docker-compose.yml | 12 ++++ .../ctc-network-policies/iptables/conf.toml | 0 .../iptables/expected-iptables-v4.txt | 32 ++++++++++ .../iptables/expected-iptables-v6.txt | 16 +++++ .../ctc-network-policies/nftables/conf.toml | 0 .../nftables/expected-nftables.txt | 24 +++++++ src/iptables/process.rs | 24 +++++++ src/nftables/process.rs | 24 +++++++ src/types.rs | 63 +++++++++++++++++++ tests/dfw.rs | 1 + tests/types.rs | 2 + 13 files changed, 206 insertions(+) create mode 100644 resources/test/docker/ctc-network-policies/conf.toml create mode 100644 resources/test/docker/ctc-network-policies/docker-compose.yml create mode 100644 resources/test/docker/ctc-network-policies/iptables/conf.toml create mode 100644 resources/test/docker/ctc-network-policies/iptables/expected-iptables-v4.txt create mode 100644 resources/test/docker/ctc-network-policies/iptables/expected-iptables-v6.txt create mode 100644 resources/test/docker/ctc-network-policies/nftables/conf.toml create mode 100644 resources/test/docker/ctc-network-policies/nftables/expected-nftables.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index 1137910f..76689215 100644 --- a/CHANGELOG.md +++ b/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]. diff --git a/resources/test/docker/ctc-network-policies/conf.toml b/resources/test/docker/ctc-network-policies/conf.toml new file mode 100644 index 00000000..1e0f36bb --- /dev/null +++ b/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" diff --git a/resources/test/docker/ctc-network-policies/docker-compose.yml b/resources/test/docker/ctc-network-policies/docker-compose.yml new file mode 100644 index 00000000..72fb97aa --- /dev/null +++ b/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: diff --git a/resources/test/docker/ctc-network-policies/iptables/conf.toml b/resources/test/docker/ctc-network-policies/iptables/conf.toml new file mode 100644 index 00000000..e69de29b diff --git a/resources/test/docker/ctc-network-policies/iptables/expected-iptables-v4.txt b/resources/test/docker/ctc-network-policies/iptables/expected-iptables-v4.txt new file mode 100644 index 00000000..ed5c577b --- /dev/null +++ b/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 diff --git a/resources/test/docker/ctc-network-policies/iptables/expected-iptables-v6.txt b/resources/test/docker/ctc-network-policies/iptables/expected-iptables-v6.txt new file mode 100644 index 00000000..f369ad52 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/resources/test/docker/ctc-network-policies/nftables/conf.toml b/resources/test/docker/ctc-network-policies/nftables/conf.toml new file mode 100644 index 00000000..e69de29b diff --git a/resources/test/docker/ctc-network-policies/nftables/expected-nftables.txt b/resources/test/docker/ctc-network-policies/nftables/expected-nftables.txt new file mode 100644 index 00000000..1a56213d --- /dev/null +++ b/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" diff --git a/src/iptables/process.rs b/src/iptables/process.rs index 9b99d7fd..e10263d4 100644 --- a/src/iptables/process.rs +++ b/src/iptables/process.rs @@ -252,6 +252,30 @@ impl Process 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, diff --git a/src/nftables/process.rs b/src/nftables/process.rs index f9a5564e..3da0fd9b 100644 --- a/src/nftables/process.rs +++ b/src/nftables/process.rs @@ -330,6 +330,30 @@ impl Process 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)) } } diff --git a/src/types.rs b/src/types.rs index 752fa43b..90f4983b 100644 --- a/src/types.rs +++ b/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::>(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::>(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, /// An optional list of rules, see /// [`ContainerToContainerRule`](struct.ContainerToContainerRule.html). /// diff --git a/tests/dfw.rs b/tests/dfw.rs index 27a94a17..01f3181f 100644 --- a/tests/dfw.rs +++ b/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"; diff --git a/tests/types.rs b/tests/types.rs index 8d646f52..476e3513 100644 --- a/tests/types.rs +++ b/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()), From d0197e51f0d5484b0d0e30aea20e53b268813122 Mon Sep 17 00:00:00 2001 From: Pit Kleyersburg Date: Fri, 5 Jan 2024 17:23:19 +0100 Subject: [PATCH 2/3] Don't test heavily outdated Docker versions anymore --- .github/workflows/it.yml | 12 ------------ README.md | 10 ++-------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/.github/workflows/it.yml b/.github/workflows/it.yml index fb0d1c18..cd756584 100644 --- a/.github/workflows/it.yml +++ b/.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: diff --git a/README.md b/README.md index 89639c1d..4e516a91 100644 --- a/README.md +++ b/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. ## Version bump policy From 164afb008f87eeb82732d25560630c6930f954c2 Mon Sep 17 00:00:00 2001 From: Pit Kleyersburg Date: Fri, 5 Jan 2024 17:25:26 +0100 Subject: [PATCH 3/3] Fix clippy lints --- src/process.rs | 2 +- tests/types.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/process.rs b/src/process.rs index 57641c88..8d5f1875 100644 --- a/src/process.rs +++ b/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 { diff --git a/tests/types.rs b/tests/types.rs index 476e3513..c1a326ce 100644 --- a/tests/types.rs +++ b/tests/types.rs @@ -596,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()); }