Skip to content

Commit

Permalink
Merge branch 'gh-90-fixsourcecidr-ipv6' into 'master'
Browse files Browse the repository at this point in the history 
Fix source-CIDR handling, specifically with IPv6

See merge request pitkley/dfwrs!124
  • Loading branch information
@pitkley
pitkley committed Sep 28, 2019
2 parents 80b5c21 + ad13919 commit ab0bea4
Show file tree
Hide file tree
Showing 9 changed files with 211 additions and 100 deletions.
3 changes: 2 additions & 1 deletion resources/test/conf-file.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ network = "network"
dst_container = "dst_container"
expose_port = 22
external_network_interface = "eni"
source_cidr = ["192.0.2.1/32", "192.0.2.2/32"]
source_cidr_v4 = ["192.0.2.1/32", "192.0.2.2/32"]
source_cidr_v6 = ["2001:db8::1/128", "2001:db8::2/128"]

[container_dnat]

Expand Down
3 changes: 2 additions & 1 deletion resources/test/conf_path/wider_world_to_container.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ network = "network"
dst_container = "dst_container"
expose_port = 22
external_network_interface = "eni"
source_cidr = ["192.0.2.1/32", "192.0.2.2/32"]
source_cidr_v4 = ["192.0.2.1/32", "192.0.2.2/32"]
source_cidr_v6 = ["2001:db8::1/128", "2001:db8::2/128"]
2 changes: 1 addition & 1 deletion resources/test/docker/02/expected-nftables.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ add chain ip6 dfw prerouting { type nat hook prerouting priority -105 ; }
add chain ip6 dfw postrouting { type nat hook postrouting priority 95 ; }
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 ip saddr $src_ip=ip daddr $dst_ip=ip meta iifname $input=bridge oifname $output=bridge meta mark set 0xdf ct state related accept "$input" == "$output"
add rule inet dfw forward ip saddr $src_ip=ip ip daddr $dst_ip=ip meta iifname $input=bridge oifname $output=bridge meta mark set 0xdf ct state related accept "$input" == "$output"
6 changes: 4 additions & 2 deletions resources/test/docker/05/conf.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@ external_network_interface = "other"
network = "dfwtest05_default"
dst_container = "dfwtest05_a_1"
expose_port = "22"
source_cidr = "192.0.2.1/32"
source_cidr_v4 = "192.0.2.1/32"
source_cidr_v6 = "2001:db8::1/128"

[[wider_world_to_container.rules]]
network = "dfwtest05_default"
dst_container = "dfwtest05_a_1"
expose_port = "25"
source_cidr = ["192.0.2.2/32", "192.0.2.3/32"]
source_cidr_v4 = ["192.0.2.2/32", "192.0.2.3/32"]
source_cidr_v6 = ["2001:db8::2/128", "2001:db8::3/128"]
12 changes: 6 additions & 6 deletions resources/test/docker/05/expected-nftables.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,12 @@ add rule ip6 dfw prerouting udp dport 53 meta iifname eni meta mark set 0xdf
add rule inet dfw forward tcp dport 443 ip daddr $dst_ip=ip meta iifname other oifname $output=bridge meta mark set 0xdf accept
add rule ip dfw prerouting tcp dport 443 meta iifname other meta mark set 0xdf dnat ${dst_ip=ip}:443
add rule ip6 dfw prerouting tcp dport 443 meta iifname other meta mark set 0xdf
add rule inet dfw forward tcp dport 22 ip saddr 192.0.2.1/32 daddr $dst_ip=ip meta iifname eni oifname $output=bridge meta mark set 0xdf accept
add rule inet dfw forward tcp dport 22 ip saddr 192.0.2.1/32 ip daddr $dst_ip=ip meta iifname eni oifname $output=bridge meta mark set 0xdf accept
add rule ip dfw prerouting tcp dport 22 ip saddr 192.0.2.1/32 meta iifname eni meta mark set 0xdf dnat ${dst_ip=ip}:22
add rule ip6 dfw prerouting tcp dport 22 ip saddr 192.0.2.1/32 meta iifname eni meta mark set 0xdf
add rule inet dfw forward tcp dport 25 ip saddr 192.0.2.2/32 daddr $dst_ip=ip meta iifname eni oifname $output=bridge meta mark set 0xdf accept
add rule inet dfw forward tcp dport 25 ip saddr 192.0.2.3/32 daddr $dst_ip=ip meta iifname eni oifname $output=bridge meta mark set 0xdf accept
add rule ip6 dfw prerouting tcp dport 22 ip6 saddr 2001:db8::1/128 meta iifname eni meta mark set 0xdf
add rule inet dfw forward tcp dport 25 ip saddr 192.0.2.2/32 ip daddr $dst_ip=ip meta iifname eni oifname $output=bridge meta mark set 0xdf accept
add rule inet dfw forward tcp dport 25 ip saddr 192.0.2.3/32 ip daddr $dst_ip=ip meta iifname eni oifname $output=bridge meta mark set 0xdf accept
add rule ip dfw prerouting tcp dport 25 ip saddr 192.0.2.2/32 meta iifname eni meta mark set 0xdf dnat ${dst_ip=ip}:25
add rule ip dfw prerouting tcp dport 25 ip saddr 192.0.2.3/32 meta iifname eni meta mark set 0xdf dnat ${dst_ip=ip}:25
add rule ip6 dfw prerouting tcp dport 25 ip saddr 192.0.2.2/32 meta iifname eni meta mark set 0xdf
add rule ip6 dfw prerouting tcp dport 25 ip saddr 192.0.2.3/32 meta iifname eni meta mark set 0xdf
add rule ip6 dfw prerouting tcp dport 25 ip6 saddr 2001:db8::2/128 meta iifname eni meta mark set 0xdf
add rule ip6 dfw prerouting tcp dport 25 ip6 saddr 2001:db8::3/128 meta iifname eni meta mark set 0xdf
176 changes: 111 additions & 65 deletions src/process.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -784,71 +784,27 @@ impl Process for WiderWorldToContainerRule {

// If source CIDRs have been specified, create the FORWARD-rules as required to
// restrict the traffic as intended.
if let Some(source_cidrs) = &self.source_cidr {
debug!(ctx.logger, "Generate extended FORWARD rules, source CIDRs were specified";
o!("args" => format!("{:?}", nft_dnat_rule),
"source_cidrs" => source_cidrs.join(", ")));
for additional_forward_rule in source_cidrs
.iter()
.map(|source_cidr| {
let mut forward_rule = nft_forward_rule.clone();
forward_rule.source_address(source_cidr);
forward_rule
})
.map(|forward_rule| forward_rule.build())
.collect::<Result<Vec<_>>>()?
{
debug!(ctx.logger, "Add FORWARD rule";
o!("part" => "wider_world_to_container",
"rule" => &additional_forward_rule));
rules.push(nftables::add_rule(
Family::Inet,
"dfw",
"forward",
&additional_forward_rule,
));
}
for additional_dnat_rule in source_cidrs
.iter()
.map(|source_cidr| {
let mut dnat_rule = nft_dnat_rule.clone();
dnat_rule.source_address(source_cidr);
dnat_rule
})
.map(|dnat_rule| dnat_rule.build())
.collect::<Result<Vec<_>>>()?
{
debug!(ctx.logger, "Add DNAT rule";
o!("part" => "wider_world_to_container",
"rule" => &additional_dnat_rule));
rules.push(nftables::add_rule(
Family::Ip,
"dfw",
"prerouting",
&additional_dnat_rule,
));
}
for additional_mark_rule in source_cidrs
.iter()
.map(|source_cidr| {
let mut mark_rule = nft_mark_rule.clone();
mark_rule.source_address(source_cidr);
mark_rule
})
.map(|dnat_rule| dnat_rule.build())
.collect::<Result<Vec<_>>>()?
{
debug!(ctx.logger, "Add mark rule";
o!("part" => "wider_world_to_container",
"rule" => &additional_mark_rule));
rules.push(nftables::add_rule(
Family::Ip6,
"dfw",
"prerouting",
&additional_mark_rule,
));
}
} else {
if let Some(source_cidrs_v4) = &self.source_cidr_v4 {
self.apply_source_cidrs_v4(
ctx,
&mut rules,
source_cidrs_v4,
nft_forward_rule.clone(),
nft_dnat_rule.clone(),
)?;
}
if let Some(source_cidrs_v6) = &self.source_cidr_v6 {
self.apply_source_cidrs_v6(
ctx,
&mut rules,
source_cidrs_v6,
nft_mark_rule.clone(),
)?;
}

// If no source CIDRs were specified, we create the default rules that allow all
// connections from any IP.
if self.source_cidr_v4.is_none() && self.source_cidr_v6.is_none() {
let forward_rule = nft_forward_rule.build()?;
debug!(ctx.logger, "Add forward rule";
o!("part" => "wider_world_to_container",
Expand Down Expand Up @@ -887,6 +843,96 @@ impl Process for WiderWorldToContainerRule {
}
}

impl WiderWorldToContainerRule {
fn apply_source_cidrs_v4(
&self,
ctx: &ProcessContext,
rules: &mut Vec<String>,
source_cidrs: &Vec<String>,
nft_forward_rule: RuleBuilder,
nft_dnat_rule: RuleBuilder,
) -> Result<()> {
debug!(ctx.logger, "Generate extended FORWARD rules, source CIDRs (IPv4) were specified";
o!("args" => format!("{:?}", nft_dnat_rule),
"source_cidrs" => source_cidrs.join(", ")));
for additional_forward_rule in source_cidrs
.iter()
.map(|source_cidr| {
let mut forward_rule = nft_forward_rule.clone();
forward_rule.source_address(source_cidr);
forward_rule
})
.map(|forward_rule| forward_rule.build())
.collect::<Result<Vec<_>>>()?
{
debug!(ctx.logger, "Add FORWARD rule";
o!("part" => "wider_world_to_container",
"rule" => &additional_forward_rule));
rules.push(nftables::add_rule(
Family::Inet,
"dfw",
"forward",
&additional_forward_rule,
));
}
for additional_dnat_rule in source_cidrs
.iter()
.map(|source_cidr| {
let mut dnat_rule = nft_dnat_rule.clone();
dnat_rule.source_address(source_cidr);
dnat_rule
})
.map(|dnat_rule| dnat_rule.build())
.collect::<Result<Vec<_>>>()?
{
debug!(ctx.logger, "Add DNAT rule";
o!("part" => "wider_world_to_container",
"rule" => &additional_dnat_rule));
rules.push(nftables::add_rule(
Family::Ip,
"dfw",
"prerouting",
&additional_dnat_rule,
));
}

Ok(())
}

fn apply_source_cidrs_v6(
&self,
ctx: &ProcessContext,
rules: &mut Vec<String>,
source_cidrs: &Vec<String>,
nft_mark_rule: RuleBuilder,
) -> Result<()> {
debug!(ctx.logger, "Generate extended prerouting rules, source CIDRs (IPv6) were specified";
o!("args" => format!("{:?}", nft_mark_rule),
"source_cidrs" => source_cidrs.join(", ")));
for additional_mark_rule in source_cidrs
.iter()
.map(|source_cidr| {
let mut mark_rule = nft_mark_rule.clone();
mark_rule.source_address_v6(source_cidr);
mark_rule
})
.map(|dnat_rule| dnat_rule.build())
.collect::<Result<Vec<_>>>()?
{
debug!(ctx.logger, "Add mark rule";
o!("part" => "wider_world_to_container",
"rule" => &additional_mark_rule));
rules.push(nftables::add_rule(
Family::Ip6,
"dfw",
"prerouting",
&additional_mark_rule,
));
}
Ok(())
}
}

impl Process for ContainerDNAT {
fn process(&self, ctx: &ProcessContext) -> Result<Option<Vec<String>>> {
if self.rules.is_some() {
Expand Down
33 changes: 24 additions & 9 deletions src/rule.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ pub struct Rule {
#[builder(setter(into))]
pub destination_address: String,
#[builder(setter(into))]
pub source_address_v6: String,
#[builder(setter(into))]
pub destination_address_v6: String,
#[builder(setter(into))]
pub protocol: String,
#[builder(setter(into))]
pub source_port: String,
Expand Down Expand Up @@ -52,16 +56,27 @@ impl RuleBuilder {
}

// Handle `ip` matches
if self.source_address.is_some() || self.destination_address.is_some() {
if let Some(source_address) = &self.source_address {
args.push("ip".to_owned());
if let Some(source_address) = &self.source_address {
args.push("saddr".to_owned());
args.push(source_address.to_owned());
}
if let Some(destination_address) = &self.destination_address {
args.push("daddr".to_owned());
args.push(destination_address.to_owned());
}
args.push("saddr".to_owned());
args.push(source_address.to_owned());
}
if let Some(destination_address) = &self.destination_address {
args.push("ip".to_owned());
args.push("daddr".to_owned());
args.push(destination_address.to_owned());
}

// Handle `ip6` matches
if let Some(source_address) = &self.source_address_v6 {
args.push("ip6".to_owned());
args.push("saddr".to_owned());
args.push(source_address.to_owned());
}
if let Some(destination_address) = &self.destination_address_v6 {
args.push("ip6".to_owned());
args.push("daddr".to_owned());
args.push(destination_address.to_owned());
}

// Handle interface-matches
Expand Down
40 changes: 35 additions & 5 deletions src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ pub struct WiderWorldToContainerRule {
/// Specific external network interface to target.
pub external_network_interface: Option<String>,

/// Source CIDRs to which incoming traffic should be restricted.
/// Source CIDRs (IPv4) to which incoming traffic should be restricted.
///
/// This can be:
///
Expand All @@ -409,12 +409,42 @@ pub struct WiderWorldToContainerRule {
/// All of the following are legal TOML fragments:
///
/// ```toml
/// source_cidr = "127.0.0.0/8"
/// source_cidr_v4 = "127.0.0.0/8"
///
/// source_cidr = ["127.0.0.0/8", "192.0.2.1/32"]
/// source_cidr _v4= ["127.0.0.0/8", "192.0.2.1/32"]
/// ```
#[serde(default, deserialize_with = "option_string_or_seq_string")]
pub source_cidr: Option<Vec<String>>,
#[serde(
default,
deserialize_with = "option_string_or_seq_string",
alias = "source_cidr"
)]
pub source_cidr_v4: Option<Vec<String>>,

/// Source CIDRs (IPv6) to which incoming traffic should be restricted.
///
/// This can be:
///
/// * a single string
///
/// * a list of strings
///
/// There is no validation whether the provided CIDRs are actually valid.
///
/// # Example
///
/// All of the following are legal TOML fragments:
///
/// ```toml
/// source_cidr_v6 = "fe80::/10"
///
/// source_cidr_v6 = ["fe80::/10", "2001:db8::/32"]
/// ```
#[serde(
default,
deserialize_with = "option_string_or_seq_string",
alias = "source_cidr"
)]
pub source_cidr_v6: Option<Vec<String>>,
}

/// Struct to hold a port definition to expose on the host/between containers.
Expand Down
Loading

0 comments on commit ab0bea4

Please sign in to comment.