Automatically manage firewall rules (iptables) for services #1

Merged
quentin merged 7 commits from add-firewall-rules into master 2020-07-04 15:16:23 +00:00
6 changed files with 129 additions and 32 deletions
Showing only changes of commit d2ae084fc1 - Show all commits

View file

@ -8,11 +8,12 @@ use serde::{Serialize, Deserialize};
use serde_lexpr::{from_str,error}; use serde_lexpr::{from_str,error};
use crate::messages; use crate::messages;
use crate::consul; use crate::consul;
use std::collections::HashSet;
#[derive(Serialize, Deserialize, Debug)] #[derive(Serialize, Deserialize, Debug)]
pub enum DiplonatParameter { pub enum DiplonatParameter {
tcp_port(Vec<u16>), tcp_port(HashSet<u16>),
udp_port(Vec<u16>) udp_port(HashSet<u16>)
} }
#[derive(Serialize, Deserialize, Debug)] #[derive(Serialize, Deserialize, Debug)]
@ -53,8 +54,8 @@ fn to_parameters(catalog: &consul::CatalogNode) -> Vec<DiplonatConsul> {
fn to_open_ports(params: &Vec<DiplonatConsul>) -> messages::PublicExposedPorts { fn to_open_ports(params: &Vec<DiplonatConsul>) -> messages::PublicExposedPorts {
let mut op = messages::PublicExposedPorts { let mut op = messages::PublicExposedPorts {
tcp_ports: Vec::new(), tcp_ports: HashSet::new(),
udp_ports: Vec::new() udp_ports: HashSet::new()
}; };
for conf in params { for conf in params {
@ -73,8 +74,8 @@ fn to_open_ports(params: &Vec<DiplonatConsul>) -> messages::PublicExposedPorts {
impl ConsulActor { impl ConsulActor {
pub fn new(url: &str, node: &str) -> Self { pub fn new(url: &str, node: &str) -> Self {
let (tx, rx) = watch::channel(messages::PublicExposedPorts{ let (tx, rx) = watch::channel(messages::PublicExposedPorts{
tcp_ports: Vec::new(), tcp_ports: HashSet::new(),
udp_ports: Vec::new() udp_ports: HashSet::new()
}); });
return Self { return Self {

View file

@ -4,10 +4,12 @@ use tokio::try_join;
use crate::consul_actor::ConsulActor; use crate::consul_actor::ConsulActor;
use crate::igd_actor::IgdActor; use crate::igd_actor::IgdActor;
use crate::environment::Environment; use crate::environment::Environment;
use crate::fw_actor::FirewallActor;
pub struct Diplonat { pub struct Diplonat {
consul: ConsulActor, consul: ConsulActor,
igd: IgdActor igd: IgdActor,
firewall: FirewallActor
} }
impl Diplonat { impl Diplonat {
@ -21,9 +23,15 @@ impl Diplonat {
&ca.rx_open_ports &ca.rx_open_ports
).await?; ).await?;
let fw = FirewallActor::new(
env.refresh_time,
&ca.rx_open_ports
).await?;
let ctx = Self { let ctx = Self {
consul: ca, consul: ca,
igd: ia igd: ia,
firewall: fw
}; };
return Ok(ctx); return Ok(ctx);
@ -32,7 +40,8 @@ impl Diplonat {
pub async fn listen(&mut self) -> Result<()> { pub async fn listen(&mut self) -> Result<()> {
try_join!( try_join!(
self.consul.listen(), self.consul.listen(),
self.igd.listen() self.igd.listen(),
self.firewall.listen()
)?; )?;
return Ok(()); return Ok(());

View file

@ -2,13 +2,7 @@ use iptables;
use regex::Regex; use regex::Regex;
use std::collections::HashSet; use std::collections::HashSet;
use std::io; use std::io;
use crate::messages;

Could you add the anyhow crate to handle errors please:

use anyhow::{Result,Context}
Could you add the anyhow crate to handle errors please: ```rust use anyhow::{Result,Context} ```
#[derive(PartialEq,Eq,Debug,Hash)]
pub struct Port {
proto: String,
number: u16,
}
#[derive(Debug)] #[derive(Debug)]
pub struct FirewallError(String); pub struct FirewallError(String);
@ -17,26 +11,34 @@ impl From<iptables::error::IPTError> for FirewallError {
fn from(error: iptables::error::IPTError) -> Self { fn from(error: iptables::error::IPTError) -> Self {
FirewallError(error.to_string()) FirewallError(error.to_string())
} }
} }

anyhow will override your Result<> object, taking only a return value, error will be generic then.
eg:

pub fn setup(ipt: &iptables::IPTables) -> Result<()> {

This tip applies for the whole document, and in any case build will fail as soon as you will have added the anyhow use statement.

anyhow will override your Result<> object, taking only a return value, error will be generic then. eg: ```rust pub fn setup(ipt: &iptables::IPTables) -> Result<()> { ``` This tip applies for the whole document, and in any case build will fail as soon as you will have added the anyhow use statement.
pub fn setup(ipt: &iptables::IPTables) -> Result<(), FirewallError> { pub fn setup(ipt: &iptables::IPTables) -> Result<(), FirewallError> {

It's very clever to put the rules in a separate chain, well done ;)

It's very clever to put the rules in a separate chain, well done ;)
ipt.new_chain("filter", "DIPLONAT")?; ipt.new_chain("filter", "DIPLONAT")?;
ipt.insert("filter", "INPUT", "-j DIPLONAT", 1)?; ipt.insert("filter", "INPUT", "-j DIPLONAT", 1)?;
Ok(()) Ok(())
} }
pub fn open_ports(ipt: &iptables::IPTables, ports: Vec<Port>) -> Result<(), FirewallError> { pub fn open_ports(ipt: &iptables::IPTables, ports: messages::PublicExposedPorts) -> Result<(), FirewallError> {
for p in ports { for p in ports.tcp_ports {

Same as before, you can just use Result<()>

Same as before, you can just use `Result<()>`
ipt.append("filter", "DIPLONAT", &format!("-p {} --dport {} -j ACCEPT", p.proto, p.number))?; ipt.append("filter", "DIPLONAT", &format!("-p tcp --dport {} -j ACCEPT", p))?;
}
for p in ports.udp_ports {
ipt.append("filter", "DIPLONAT", &format!("-p udp --dport {} -j ACCEPT", p))?;
} }
Ok(()) Ok(())
} }
pub fn get_opened_ports(ipt: &iptables::IPTables) -> Result<HashSet<Port>, FirewallError> { pub fn get_opened_ports(ipt: &iptables::IPTables) -> Result<messages::PublicExposedPorts, FirewallError> {
let mut opened_ports: HashSet<Port> = HashSet::new(); let mut ports = messages::PublicExposedPorts {

Same as before, you can just use Result<messages::PublicExposedPorts>

Same as before, you can just use `Result<messages::PublicExposedPorts>`
tcp_ports: HashSet::new(),
udp_ports: HashSet::new()
};
let list = ipt.list("filter", "DIPLONAT")?; let list = ipt.list("filter", "DIPLONAT")?;
let re = Regex::new(r"\-A.*? \-p (\w+).*\-\-dport (\d+).*?\-j ACCEPT").unwrap(); let re = Regex::new(r"\-A.*? \-p (\w+).*\-\-dport (\d+).*?\-j ACCEPT").unwrap();
@ -50,13 +52,18 @@ pub fn get_opened_ports(ipt: &iptables::IPTables) -> Result<HashSet<Port>, Firew
let proto = String::from(raw_proto.as_str()); let proto = String::from(raw_proto.as_str());
let number = String::from(raw_port.as_str()).parse::<u16>().unwrap(); let number = String::from(raw_port.as_str()).parse::<u16>().unwrap();
opened_ports.insert( Port { proto, number } ); if proto == "tcp" {
ports.tcp_ports.insert(number);
} else {
ports.udp_ports.insert(number);
}
}, },
_ => {} _ => {}
} }
} }
Ok(opened_ports) Ok(ports)
} }
pub fn cleanup(ipt: &iptables::IPTables) -> Result<(), FirewallError> { pub fn cleanup(ipt: &iptables::IPTables) -> Result<(), FirewallError> {

80
src/fw_actor.rs Normal file
View file

@ -0,0 +1,80 @@
use igd::aio::*;
use igd::PortMappingProtocol;
use std::net::SocketAddrV4;
use log::*;
use anyhow::{Result, Context};
use tokio::{
select,
sync::watch,
time::{
self,
Duration
}};
use iptables;
use crate::messages;
use crate::fw;
use std::collections::HashSet;
pub struct FirewallActor {
ipt: iptables::IPTables,
rx_ports: watch::Receiver<messages::PublicExposedPorts>,
last_ports: messages::PublicExposedPorts,
refresh: Duration
}
impl FirewallActor {

Replace with ?

Replace with `?`
pub async fn new(_refresh: Duration, rxp: &watch::Receiver<messages::PublicExposedPorts>) -> Result<Self> {
let ctx = Self {
ipt: iptables::new(false).unwrap(),
rx_ports: rxp.clone(),

Here also you can put a ? :)

Here also you can put a `?` :)
last_ports: messages::PublicExposedPorts::new(),
refresh: _refresh,
};
fw::setup(&ctx.ipt).expect("iptables setup failed");
return Ok(ctx);
}
pub async fn listen(&mut self) -> Result<()> {
let mut interval = time::interval(self.refresh);
loop {
// 1. Wait for an event
let new_ports = select! {
Some(ports) = self.rx_ports.recv() => Some(ports),
_ = interval.tick() => None,
else => return Ok(()) // Sender dropped, terminate loop.
};
// 2. Update last ports if needed
if let Some(p) = new_ports { self.last_ports = p; }
darkgallium marked this conversation as resolved
Review

By putting ? everywhere, we can handle all errors here.
Currently, this line can't be called as there is no way that do_fw_update returns an Err as inside the function, you never returns an Err as unwrap makes the app panic ;)

By putting `?` everywhere, we can handle all errors here. Currently, this line can't be called as there is no way that `do_fw_update` returns an `Err` as inside the function, you never returns an `Err` as `unwrap` makes the app panic ;)
// 3. Update firewall rules
match self.do_fw_update().await {
Ok(()) => debug!("Successfully updated firewall rules"),
Err(e) => error!("An error occured while updating firewall rules. {}", e),
}

You should replace unwrap ;)

You should replace `unwrap` ;)
}
}
quentin marked this conversation as resolved
Review

Nice idea the set difference ;)

Nice idea the set difference ;)
Review

Thanks a lot :)

Thanks a lot :)
pub async fn do_fw_update(&self) -> Result<()> {
let curr_opened_ports = fw::get_opened_ports(&self.ipt).unwrap();
let diff_tcp = self.last_ports.tcp_ports.difference(&curr_opened_ports.tcp_ports).copied().collect::<HashSet<u16>>();
let diff_udp = self.last_ports.udp_ports.difference(&curr_opened_ports.udp_ports).copied().collect::<HashSet<u16>>();
quentin marked this conversation as resolved
Review

I see, maybe we could add later logic to close ports but I also like the idea to do the things incrementally.
So let's focus on stabilizing the current features and see how it runs in production :)

I see, maybe we could add later logic to close ports but I also like the idea to do the things incrementally. So let's focus on stabilizing the current features and see how it runs in production :)

Here also ;)

Here also ;)
let ports_to_open = messages::PublicExposedPorts {
tcp_ports: diff_tcp,
udp_ports: diff_udp
};
fw::open_ports(&self.ipt, ports_to_open).unwrap();
return Ok(());
}
}

View file

@ -5,6 +5,7 @@ mod consul_actor;
mod igd_actor; mod igd_actor;
mod diplonat; mod diplonat;
mod fw; mod fw;
mod fw_actor;
use iptables; use iptables;
use log::*; use log::*;
@ -15,9 +16,6 @@ async fn main() {
pretty_env_logger::init(); pretty_env_logger::init();
info!("Starting Diplonat"); info!("Starting Diplonat");
let ipt = iptables::new(false).unwrap();
fw::setup(&ipt).expect("iptables setup failed");
let mut diplo = Diplonat::new().await.expect("Setup failed"); let mut diplo = Diplonat::new().await.expect("Setup failed");
diplo.listen().await.expect("A runtime error occured"); diplo.listen().await.expect("A runtime error occured");
} }

View file

@ -1,14 +1,16 @@
#[derive(Debug, Clone)] use std::collections::HashSet;
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct PublicExposedPorts { pub struct PublicExposedPorts {
pub tcp_ports: Vec<u16>, pub tcp_ports: HashSet<u16>,
pub udp_ports: Vec<u16> pub udp_ports: HashSet<u16>
} }
impl PublicExposedPorts { impl PublicExposedPorts {
pub fn new() -> Self { pub fn new() -> Self {
return Self { return Self {
tcp_ports: Vec::new(), tcp_ports: HashSet::new(),
udp_ports: Vec::new() udp_ports: HashSet::new()
} }
} }
} }