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
10 changed files with 334 additions and 38 deletions

148
Cargo.lock generated
View file

@ -1,5 +1,14 @@
# This file is automatically @generated by Cargo. # This file is automatically @generated by Cargo.
# It is not intended for manual editing. # It is not intended for manual editing.
[[package]]
name = "aho-corasick"
version = "0.6.10"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "81ce3d38065e618af2d7b77e10c5ad9a069859b4be3c2250f674af3840d9c8a5"
dependencies = [
"memchr",
]
[[package]] [[package]]
name = "aho-corasick" name = "aho-corasick"
version = "0.7.10" version = "0.7.10"
@ -49,6 +58,12 @@ version = "0.11.0"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b41b7ea54a0c9d92199de89e20e58d49f02f8e699814ef3fdf266f6f748d15c7" checksum = "b41b7ea54a0c9d92199de89e20e58d49f02f8e699814ef3fdf266f6f748d15c7"
[[package]]
name = "bitflags"
version = "0.4.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8dead7461c1127cf637931a1e50934eb6eee8bff2f74433ac7909e9afcee04a3"
[[package]] [[package]]
name = "bitflags" name = "bitflags"
version = "1.2.1" version = "1.2.1"
@ -127,8 +142,10 @@ dependencies = [
"anyhow", "anyhow",
"futures", "futures",
"igd", "igd",
"iptables",
"log", "log",
"pretty_env_logger", "pretty_env_logger",
"regex 1.3.7",
"reqwest", "reqwest",
"serde", "serde",
"serde-lexpr", "serde-lexpr",
@ -160,7 +177,7 @@ dependencies = [
"atty", "atty",
"humantime", "humantime",
"log", "log",
"regex", "regex 1.3.7",
"termcolor", "termcolor",
] ]
@ -197,7 +214,7 @@ version = "0.3.3"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2e9763c69ebaae630ba35f74888db465e49e259ba1bc0eda7d06f4a067615d82" checksum = "2e9763c69ebaae630ba35f74888db465e49e259ba1bc0eda7d06f4a067615d82"
dependencies = [ dependencies = [
"bitflags", "bitflags 1.2.1",
"fuchsia-zircon-sys", "fuchsia-zircon-sys",
] ]
@ -492,6 +509,17 @@ dependencies = [
"libc", "libc",
] ]
[[package]]
name = "iptables"
version = "0.2.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f7910549cb022e3112eea631a4c3e62523281b6c61024b2c3a8d61da620010de"
dependencies = [
"lazy_static 0.2.11",
"nix",
"regex 0.2.11",
]
[[package]] [[package]]
name = "itoa" name = "itoa"
version = "0.4.5" version = "0.4.5"
@ -517,6 +545,12 @@ dependencies = [
"winapi-build", "winapi-build",
] ]
[[package]]
name = "lazy_static"
version = "0.2.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "76f033c7ad61445c5b347c7382dd1237847eb1bce590fe50365dcb33d546be73"
[[package]] [[package]]
name = "lazy_static" name = "lazy_static"
version = "1.4.0" version = "1.4.0"
@ -548,9 +582,9 @@ dependencies = [
[[package]] [[package]]
name = "libc" name = "libc"
version = "0.2.66" version = "0.2.70"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d515b1f41455adea1313a4a2ac8a8a477634fbae63cc6100e3aebb207ce61558" checksum = "3baa92041a6fec78c687fa0cc2b3fae8884f743d672cf551bed1d6dac6988d0f"
[[package]] [[package]]
name = "log" name = "log"
@ -626,7 +660,7 @@ version = "0.2.3"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4b2df1a4c22fd44a62147fd8f13dd0f95c9d8ca7b2610299b2a2f9cf8964274e" checksum = "4b2df1a4c22fd44a62147fd8f13dd0f95c9d8ca7b2610299b2a2f9cf8964274e"
dependencies = [ dependencies = [
"lazy_static", "lazy_static 1.4.0",
"libc", "libc",
"log", "log",
"openssl", "openssl",
@ -649,6 +683,20 @@ dependencies = [
"winapi 0.3.8", "winapi 0.3.8",
] ]
[[package]]
name = "nix"
version = "0.7.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a0d95c5fa8b641c10ad0b8887454ebaafa3c92b5cd5350f8fc693adafd178e7b"
dependencies = [
"bitflags 0.4.0",
"cfg-if",
"libc",
"rustc_version",
"semver",
"void",
]
[[package]] [[package]]
name = "nom" name = "nom"
version = "4.2.3" version = "4.2.3"
@ -671,10 +719,10 @@ version = "0.10.28"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "973293749822d7dd6370d6da1e523b0d1db19f06c459134c658b2a4261378b52" checksum = "973293749822d7dd6370d6da1e523b0d1db19f06c459134c658b2a4261378b52"
dependencies = [ dependencies = [
"bitflags", "bitflags 1.2.1",
"cfg-if", "cfg-if",
"foreign-types", "foreign-types",
"lazy_static", "lazy_static 1.4.0",
"libc", "libc",
"openssl-sys", "openssl-sys",
] ]
@ -889,16 +937,38 @@ version = "0.1.56"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "2439c63f3f6139d1b57529d16bc3b8bb855230c8efcc5d3a896c8bea7c3b1e84" checksum = "2439c63f3f6139d1b57529d16bc3b8bb855230c8efcc5d3a896c8bea7c3b1e84"
[[package]]
name = "regex"
version = "0.2.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9329abc99e39129fcceabd24cf5d85b4671ef7c29c50e972bc5afe32438ec384"
dependencies = [
"aho-corasick 0.6.10",
"memchr",
"regex-syntax 0.5.6",
"thread_local 0.3.6",
"utf8-ranges",
]
[[package]] [[package]]
name = "regex" name = "regex"
version = "1.3.7" version = "1.3.7"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a6020f034922e3194c711b82a627453881bc4682166cabb07134a10c26ba7692" checksum = "a6020f034922e3194c711b82a627453881bc4682166cabb07134a10c26ba7692"
dependencies = [ dependencies = [
"aho-corasick", "aho-corasick 0.7.10",
"memchr", "memchr",
"regex-syntax", "regex-syntax 0.6.17",
"thread_local", "thread_local 1.0.1",
]
[[package]]
name = "regex-syntax"
version = "0.5.6"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7d707a4fa2637f2dca2ef9fd02225ec7661fe01a53623c1e6515b6916511f7a7"
dependencies = [
"ucd-util",
] ]
[[package]] [[package]]
@ -932,7 +1002,7 @@ dependencies = [
"hyper", "hyper",
"hyper-tls", "hyper-tls",
"js-sys", "js-sys",
"lazy_static", "lazy_static 1.4.0",
"log", "log",
"mime", "mime",
"mime_guess", "mime_guess",
@ -952,6 +1022,15 @@ dependencies = [
"winreg", "winreg",
] ]
[[package]]
name = "rustc_version"
version = "0.1.7"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c5f5376ea5e30ce23c03eb77cbe4962b988deead10910c372b226388b594c084"
dependencies = [
"semver",
]
[[package]] [[package]]
name = "ryu" name = "ryu"
version = "1.0.2" version = "1.0.2"
@ -964,7 +1043,7 @@ version = "0.1.17"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "507a9e6e8ffe0a4e0ebb9a10293e62fdf7657c06f1b8bb07a8fcf697d2abf295" checksum = "507a9e6e8ffe0a4e0ebb9a10293e62fdf7657c06f1b8bb07a8fcf697d2abf295"
dependencies = [ dependencies = [
"lazy_static", "lazy_static 1.4.0",
"winapi 0.3.8", "winapi 0.3.8",
] ]
@ -989,6 +1068,12 @@ dependencies = [
"core-foundation-sys", "core-foundation-sys",
] ]
[[package]]
name = "semver"
version = "0.1.20"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d4f410fedcf71af0345d7607d246e7ad15faaadd49d240ee3b24e5dc21a820ac"
[[package]] [[package]]
name = "serde" name = "serde"
version = "1.0.107" version = "1.0.107"
@ -1094,13 +1179,22 @@ dependencies = [
"winapi-util", "winapi-util",
] ]
[[package]]
name = "thread_local"
version = "0.3.6"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c6b53e329000edc2b34dbe8545fd20e55a333362d0a321909685a19bd28c3f1b"
dependencies = [
"lazy_static 1.4.0",
]
[[package]] [[package]]
name = "thread_local" name = "thread_local"
version = "1.0.1" version = "1.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "d40c6d1b69745a6ec6fb1ca717914848da4b44ae29d9b3080cbee91d72a69b14" checksum = "d40c6d1b69745a6ec6fb1ca717914848da4b44ae29d9b3080cbee91d72a69b14"
dependencies = [ dependencies = [
"lazy_static", "lazy_static 1.4.0",
] ]
[[package]] [[package]]
@ -1116,14 +1210,14 @@ dependencies = [
[[package]] [[package]]
name = "tokio" name = "tokio"
version = "0.2.11" version = "0.2.21"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8fdd17989496f49cdc57978c96f0c9fe5e4a58a8bddc6813c449a4624f6a030b" checksum = "d099fa27b9702bed751524694adbe393e18b36b204da91eb1cbbbbb4a5ee2d58"
dependencies = [ dependencies = [
"bytes 0.5.4", "bytes 0.5.4",
"fnv", "fnv",
"iovec", "iovec",
"lazy_static", "lazy_static 1.4.0",
"memchr", "memchr",
"mio", "mio",
"pin-project-lite", "pin-project-lite",
@ -1178,6 +1272,12 @@ version = "0.2.2"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e604eb7b43c06650e854be16a2a03155743d3752dd1c943f6829e26b7a36e382" checksum = "e604eb7b43c06650e854be16a2a03155743d3752dd1c943f6829e26b7a36e382"
[[package]]
name = "ucd-util"
version = "0.1.8"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c85f514e095d348c279b1e5cd76795082cf15bd59b93207832abe0b1d8fed236"
[[package]] [[package]]
name = "unicase" name = "unicase"
version = "2.6.0" version = "2.6.0"
@ -1239,6 +1339,12 @@ dependencies = [
"percent-encoding 2.1.0", "percent-encoding 2.1.0",
] ]
[[package]]
name = "utf8-ranges"
version = "1.0.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b4ae116fef2b7fea257ed6440d3cfcff7f190865f170cdad00bb6465bf18ecba"
[[package]] [[package]]
name = "vcpkg" name = "vcpkg"
version = "0.2.8" version = "0.2.8"
@ -1257,6 +1363,12 @@ version = "0.9.1"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "078775d0255232fb988e6fccf26ddc9d1ac274299aaedcedce21c6f72cc533ce" checksum = "078775d0255232fb988e6fccf26ddc9d1ac274299aaedcedce21c6f72cc533ce"
[[package]]
name = "void"
version = "1.0.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d"
[[package]] [[package]]
name = "want" name = "want"
version = "0.3.0" version = "0.3.0"
@ -1292,7 +1404,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "11cdb95816290b525b32587d76419facd99662a07e59d3cdb560488a819d9a45" checksum = "11cdb95816290b525b32587d76419facd99662a07e59d3cdb560488a819d9a45"
dependencies = [ dependencies = [
"bumpalo", "bumpalo",
"lazy_static", "lazy_static 1.4.0",
"log", "log",
"proc-macro2", "proc-macro2",
"quote", "quote",
@ -1447,7 +1559,7 @@ version = "0.7.0"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3c1cb601d29fe2c2ac60a2b2e5e293994d87a1f6fa9687a31a15270f909be9c2" checksum = "3c1cb601d29fe2c2ac60a2b2e5e293994d87a1f6fa9687a31a15270f909be9c2"
dependencies = [ dependencies = [
"bitflags", "bitflags 1.2.1",
] ]
[[package]] [[package]]

View file

@ -11,9 +11,11 @@ reqwest = { version = "0.10", features = ["json"] }
igd = { version = "0.10.0", features = ["aio"] } igd = { version = "0.10.0", features = ["aio"] }
log = "0.4" log = "0.4"
pretty_env_logger = "0.4" pretty_env_logger = "0.4"
tokio = "0.2.11" tokio = "0.2"
futures = "0.3.5" futures = "0.3.5"
serde = { version = "1.0.107", features = ["derive"] } serde = { version = "1.0.107", features = ["derive"] }
serde_json = "1.0.53" serde_json = "1.0.53"
serde-lexpr = "0.1.1" serde-lexpr = "0.1.1"
anyhow = "1.0.28" anyhow = "1.0.28"
iptables = "0.2.2"
regex = "1"

View file

@ -18,6 +18,6 @@ COPY ./src ./src
RUN cargo build --release RUN cargo build --release
FROM debian:bullseye-slim FROM debian:bullseye-slim
RUN apt-get update && apt-get install -y libssl1.1 RUN apt-get update && apt-get install -y libssl1.1 iptables
COPY --from=builder /srv/target/release/diplonat /usr/local/sbin/diplonat COPY --from=builder /srv/target/release/diplonat /usr/local/sbin/diplonat
CMD ["/usr/local/sbin/diplonat"] CMD ["/usr/local/sbin/diplonat"]

View file

@ -4,7 +4,7 @@ Diplonat
## Feature set ## Feature set
* [X] (Re)Configure NAT via UPNP/IGD (prio: high) * [X] (Re)Configure NAT via UPNP/IGD (prio: high)
* [ ] (Re)Configure nftable (prio: low) * [X] (Re)Configure iptables (prio: low)
* [ ] (Re)Configure DNS via ??? (prio: low) * [ ] (Re)Configure DNS via ??? (prio: low)
## Understand scope ## Understand scope
@ -17,11 +17,24 @@ Diplonat
## Operate ## Operate
You need to add the following to your nomad config file :
```
client {
[...]
options {
docker.privileged.enabled = "true"
darkgallium marked this conversation as resolved
Review

Uuuuh nicely spot, I did not think to it but it makes sense :)
Thanks for testing the whole pipeline!

Uuuuh nicely spot, I did not think to it but it makes sense :) Thanks for testing the whole pipeline!
}
}
```
```bash ```bash
cargo build cargo build
consul agent -dev # in a separate terminal consul agent -dev # in a separate terminal
# adapt following values to your configuratio # adapt following values to your configuration
export DIPLONAT_PRIVATE_IP="192.168.0.18" export DIPLONAT_PRIVATE_IP="192.168.0.18"
export DIPLONAT_REFRESH_TIME="60" export DIPLONAT_REFRESH_TIME="60"
export DIPLONAT_EXPIRATION_TIME="300" export DIPLONAT_EXPIRATION_TIME="300"

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

@ -1,13 +1,14 @@
use anyhow::Result; use anyhow::Result;
use log::*;
use tokio::try_join; 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 +22,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 +39,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(());

81
src/fw.rs Normal file
View file

@ -0,0 +1,81 @@
use iptables;
use regex::Regex;
use std::collections::HashSet;
use crate::messages;
use anyhow::{Result,Context};

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} ```
use log::*;
pub fn setup(ipt: &iptables::IPTables) -> Result<()> {
// ensure we start from a clean state without any rule already set
cleanup(ipt)?;
ipt.new_chain("filter", "DIPLONAT").context("Failed to create new chain")?;
ipt.insert_unique("filter", "INPUT", "-j DIPLONAT", 1).context("Failed to insert jump rule")?;

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.
Ok(())
}

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 ;)
pub fn open_ports(ipt: &iptables::IPTables, ports: messages::PublicExposedPorts) -> Result<()> {
for p in ports.tcp_ports {
ipt.append("filter", "DIPLONAT", &format!("-p tcp --dport {} -j ACCEPT", p)).context("Failed to insert port rule")?;
}
for p in ports.udp_ports {
ipt.append("filter", "DIPLONAT", &format!("-p udp --dport {} -j ACCEPT", p)).context("Failed to insert port rule")?;
}

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

Same as before, you can just use `Result<()>`
Ok(())
}
pub fn get_opened_ports(ipt: &iptables::IPTables) -> Result<messages::PublicExposedPorts> {
let mut ports = messages::PublicExposedPorts {
tcp_ports: HashSet::new(),
udp_ports: HashSet::new()
};
let list = ipt.list("filter", "DIPLONAT")?;
let re = Regex::new(r"\-A.*? \-p (\w+).*\-\-dport (\d+).*?\-j ACCEPT").context("Regex matching open ports encountered an unexpected rule")?;

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

Same as before, you can just use `Result<messages::PublicExposedPorts>`
for i in list {
let caps = re.captures(&i);
match caps {
Some(c) => {
if let (Some(raw_proto), Some(raw_port)) = (c.get(1), c.get(2)) {

A convention we chose with LX (we should document it, mea culpa) is to use the anyhow crate and to handle errors without crashing whenever possible but forwarding it "higher" in the call stack.

The idea is the caller can choose to crash or to bypass the error and then your code can be used as a handful library. So, as your return type is already a Result, you can return Ok() or, thanks to anyhow, any kind of error. You can simply rewrite your statement as follow (as soon as you have added anyhow):

let re = Regex::new(r"-A.*? -p (w+).*--dport (d+).*?-j ACCEPT")?

To help debugging, you can add a context to your error:

let re = Regex::new(r"-A.*? -p (w+).*--dport (d+).*?-j ACCEPT").context("Regex matching open ports encountered an unexpected rule")?

As a more general rule, do not use:

  • expect
  • unwrap

Instead use:

  • ?
  • context
  • handle error in place if it's possible
A convention we chose with LX (we should document it, mea culpa) is to use the anyhow crate and to handle errors without crashing whenever possible but forwarding it "higher" in the call stack. The idea is the caller can choose to crash or to bypass the error and then your code can be used as a handful library. So, as your return type is already a `Result`, you can return `Ok()` or, thanks to anyhow, any kind of error. You can simply rewrite your statement as follow (as soon as you have added anyhow): ```rust let re = Regex::new(r"-A.*? -p (w+).*--dport (d+).*?-j ACCEPT")? ``` To help debugging, you can add a context to your error: ```rust let re = Regex::new(r"-A.*? -p (w+).*--dport (d+).*?-j ACCEPT").context("Regex matching open ports encountered an unexpected rule")? ``` As a more general rule, do not use: - `expect` - `unwrap` Instead use: - `?` - `context` - handle error in place if it's possible
let proto = String::from(raw_proto.as_str());
let number = String::from(raw_port.as_str()).parse::<u16>()?;
if proto == "tcp" {
ports.tcp_ports.insert(number);

Lines 50, 51 and 54 you can replace unwrap by ?

Lines 50, 51 and 54 you can replace `unwrap` by `?`
} else {
ports.udp_ports.insert(number);
}
} else {
error!("Unexpected rule found in DIPLONAT chain")
}
},
_ => {}
}
}
Ok(ports)
}
pub fn cleanup(ipt: &iptables::IPTables) -> Result<()> {
if ipt.chain_exists("filter", "DIPLONAT")? {
ipt.flush_chain("filter", "DIPLONAT").context("Failed to flush the DIPLONAT chain")?;

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


Moreover, I see that you never call this function.
What about calling it on the setup, so if the app reboots at least it will start with a clean state?
Even more agressive, could we not just simply regenerates the whole chain on every change?
So, no difference to compute or ports to close?

I believe that on each modification to iptables, you have to flush the whole set of rules,
so if we could do a single "commit" to the kernel with a brand new chain it could be great.

In practise:

  • If it's two lines to change, it's easy and takes 5 minutes, you can do the modification
  • Otherwise, let's keep ip for a next version
Same as before, you can just use `Result<()>` --- Moreover, I see that you never call this function. What about calling it on the setup, so if the app reboots at least it will start with a clean state? Even more agressive, could we not just simply regenerates the whole chain on every change? So, no difference to compute or ports to close? I believe that on each modification to iptables, you have to flush the whole set of rules, so if we could do a single "commit" to the kernel with a brand new chain it could be great. In practise: - If it's two lines to change, it's easy and takes 5 minutes, you can do the modification - Otherwise, let's keep ip for a next version
if ipt.exists("filter", "INPUT", "-j DIPLONAT")? {
ipt.delete("filter", "INPUT", "-j DIPLONAT").context("Failed to delete jump rule")?;
}
ipt.delete_chain("filter", "DIPLONAT").context("Failed to delete chain")?;
}
Ok(())
}

75
src/fw_actor.rs Normal file
View file

@ -0,0 +1,75 @@
use anyhow::Result;
use tokio::{
select,
sync::watch,
time::{
self,
Duration
}};
use log::*;
use iptables;
use crate::messages;
use crate::fw;
use std::collections::HashSet;
pub struct FirewallActor {
pub ipt: iptables::IPTables,
rx_ports: watch::Receiver<messages::PublicExposedPorts>,
last_ports: messages::PublicExposedPorts,
refresh: Duration
}
impl FirewallActor {
pub async fn new(_refresh: Duration, rxp: &watch::Receiver<messages::PublicExposedPorts>) -> Result<Self> {
let ctx = Self {
ipt: iptables::new(false)?,

Replace with ?

Replace with `?`
rx_ports: rxp.clone(),
last_ports: messages::PublicExposedPorts::new(),
refresh: _refresh,
};
fw::setup(&ctx.ipt)?;

Here also you can put a ? :)

Here also you can put a `?` :)
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; }
// 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),
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 ;)
}
}
}
pub async fn do_fw_update(&self) -> Result<()> {
let curr_opened_ports = fw::get_opened_ports(&self.ipt)?;

You should replace unwrap ;)

You should replace `unwrap` ;)
let diff_tcp = self.last_ports.tcp_ports.difference(&curr_opened_ports.tcp_ports).copied().collect::<HashSet<u16>>();
quentin marked this conversation as resolved
Review

Nice idea the set difference ;)

Nice idea the set difference ;)
Review

Thanks a lot :)

Thanks a lot :)
let diff_udp = self.last_ports.udp_ports.difference(&curr_opened_ports.udp_ports).copied().collect::<HashSet<u16>>();
let ports_to_open = messages::PublicExposedPorts {
tcp_ports: diff_tcp,
udp_ports: diff_udp
};
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 :)
fw::open_ports(&self.ipt, ports_to_open)?;

Here also ;)

Here also ;)
return Ok(());
}
}

View file

@ -4,6 +4,8 @@ mod consul;
mod consul_actor; mod consul_actor;
mod igd_actor; mod igd_actor;
mod diplonat; mod diplonat;
mod fw;
mod fw_actor;
use log::*; use log::*;
use diplonat::Diplonat; use diplonat::Diplonat;

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()
} }
} }
} }