Skip to content

Commit

Permalink
chore(riklet): improve security on iptables
Browse files Browse the repository at this point in the history
* Accept only external request and target TAP interfaces of microVM.
  Previously we were accepting all requests and applying conntrack on
it, it was unsafe.

Signed-off-by: AlexandreBrg <burgoni@pm.me>
  • Loading branch information
alexandrebrg committed May 12, 2023
1 parent 42a3754 commit a3224bc
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 34 deletions.
8 changes: 4 additions & 4 deletions docs/src/reference/riklet.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ To run riklet with FAAS configuration.

```bash
sudo riklet --kernel-path ${KERNEL_LOCATION} \
--ifnet ${IFACE} \
--ifnet-ip ${IFACE_IP} \
--iface ${IFACE} \
--iface-ip ${IFACE_IP} \
```

> The firecracker binary location is determined by the following order:
Expand All @@ -33,7 +33,7 @@ Exemple:

```bash
sudo riklet --kernel-path ./vmlinux.bin \
--ifnet wlp2s0 \
--ifnet-ip 192.168.1.84 \
--iface wlp2s0 \
--iface-ip 192.168.1.84 \
--script-path ./scripts/setup-host-tap.sh
```
8 changes: 4 additions & 4 deletions riklet/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ To run riklet with FAAS configuration.

```bash
sudo riklet --kernel-path ${KERNEL_LOCATION} \
--ifnet ${IFACE} \
--ifnet-ip ${IFACE_IP} \
--iface ${IFACE} \
--iface-ip ${IFACE_IP} \
```

> The firecracker binary location is determined by the following order:
Expand All @@ -68,8 +68,8 @@ Exemple:

```bash
sudo riklet --kernel-path ./vmlinux.bin \
--ifnet wlp2s0 \
--ifnet-ip 192.168.1.84
--iface wlp2s0 \
--iface-ip 192.168.1.84
```

You should see something like that :
Expand Down
1 change: 0 additions & 1 deletion riklet/src/cli/function_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ impl FnConfiguration {
None => get_default_iface()?,
};
Ok(FnConfiguration {
firecracker_location: opts.firecracker_path,
kernel_location: opts.kernel_path,
gateway_ip,
iface,
Expand Down
2 changes: 1 addition & 1 deletion riklet/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ impl Riklet {
let fn_configuration =
FnConfiguration::load().map_err(|e| RikletError::InvalidInput(e.to_string()))?;

let mut global_runtime_network = GlobalRuntimeNetwork::new(fn_configuration.gateway_ip)
let mut global_runtime_network = GlobalRuntimeNetwork::new(fn_configuration.iface)
.map_err(|e| RikletError::NetworkError(NetworkError::IptablesError(e)))?;
global_runtime_network
.init()
Expand Down
13 changes: 11 additions & 2 deletions riklet/src/runtime/network/function_network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,15 @@ impl FunctionRuntimeNetwork {
table: Table::Filter,
chain: Chain::Forward,
};
let filter_conntrack = Rule {
chain: Chain::Forward,
table: Table::Filter,
rule: format!(
"-m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT -o {}",
self.tap_name()?
),
};
rules.push(filter_conntrack);
rules.push(rule);
// port mapping
for (exposed_port, internal_port) in self.port_mapping.iter() {
Expand Down Expand Up @@ -214,7 +223,7 @@ mod tests {

use super::FunctionRuntimeNetwork;

const GATEWAY_MOCK: Ipv4Addr = Ipv4Addr::new(192, 168, 0, 1);
const GATEWAY_MOCK: &str = "eth0";

fn open_tap_shell(iface_name: &str) -> Result<(), String> {
let tap_output = Command::new("ip")
Expand Down Expand Up @@ -281,7 +290,7 @@ mod tests {
#[tokio::test]
#[serial]
async fn apply_exposure_network_routing() {
let mut network = GlobalRuntimeNetwork::new(GATEWAY_MOCK).unwrap();
let mut network = GlobalRuntimeNetwork::new(GATEWAY_MOCK.to_string()).unwrap();
let result = network.init().await;
assert!(result.is_ok());

Expand Down
29 changes: 7 additions & 22 deletions riklet/src/runtime/network/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use async_trait::async_trait;
use once_cell::sync::Lazy;
use shared::utils::ip_allocator::IpAllocator;
use std::fmt::Debug;
use std::net::Ipv4Addr;
use std::sync::Mutex;
use thiserror::Error;

Expand Down Expand Up @@ -58,13 +57,11 @@ pub struct GlobalRuntimeNetwork {
iptables: Iptables,

/// Name of the interface that will be used as the gateway for the network
gateway_iface: Ipv4Addr,
gateway_iface: String,
}

impl GlobalRuntimeNetwork {
pub fn new(
gateway_iface: Ipv4Addr,
) -> std::result::Result<GlobalRuntimeNetwork, IptablesError> {
pub fn new(gateway_iface: String) -> std::result::Result<GlobalRuntimeNetwork, IptablesError> {
Ok(GlobalRuntimeNetwork {
iptables: Iptables::new(true)?,
gateway_iface,
Expand Down Expand Up @@ -122,17 +119,9 @@ impl RuntimeNetwork for GlobalRuntimeNetwork {
rule: format!("-o {} -j MASQUERADE", self.gateway_iface),
};

let filter_conntrack = Rule {
chain: Chain::Forward,
table: Table::Filter,
rule: "-m conntrack --ctstate RELATED,ESTABLISHED -j ACCEPT".to_string(),
};
self.iptables
.create(&nat_masquerade)
.map_err(NetworkError::IptablesError)?;
self.iptables
.create(&filter_conntrack)
.map_err(NetworkError::IptablesError)?;
Ok(())
}

Expand All @@ -151,12 +140,12 @@ mod tests {
use crate::runtime::network::{GlobalRuntimeNetwork, RuntimeNetwork};
use serial_test::serial;

const GATEWAY_MOCK: Ipv4Addr = Ipv4Addr::new(192, 168, 0, 1);
const GATEWAY_MOCK: &str = "eth0";

#[tokio::test]
#[serial]
async fn test_network_init_ok() {
let mut network = GlobalRuntimeNetwork::new(GATEWAY_MOCK).unwrap();
let mut network = GlobalRuntimeNetwork::new(GATEWAY_MOCK.to_string()).unwrap();
let result = network.init().await;
assert!(result.is_ok());
let result = network.destroy().await;
Expand All @@ -166,7 +155,7 @@ mod tests {
#[tokio::test]
#[serial]
async fn test_network_init_drop() {
let mut network = GlobalRuntimeNetwork::new(GATEWAY_MOCK).unwrap();
let mut network = GlobalRuntimeNetwork::new(GATEWAY_MOCK.to_string()).unwrap();
let result = network.init().await;
assert!(result.is_ok());

Expand Down Expand Up @@ -213,19 +202,15 @@ mod tests {
#[tokio::test]
#[serial]
async fn test_multiple_global_network_fails() {
let mut network = GlobalRuntimeNetwork::new(GATEWAY_MOCK).unwrap();
let mut network = GlobalRuntimeNetwork::new(GATEWAY_MOCK.to_string()).unwrap();
let result = network.init().await;
assert!(result.is_ok());

let mut network2 = GlobalRuntimeNetwork::new(GATEWAY_MOCK).unwrap();
let mut network2 = GlobalRuntimeNetwork::new(GATEWAY_MOCK.to_string()).unwrap();
let result = network2.init().await;
assert!(result.is_err());

let result = network.destroy().await;
assert!(result.is_ok());
}

fn test_to_string_gateway() {
assert_eq!(GATEWAY_MOCK.to_string(), "192.168.0.1".to_string());
}
}

0 comments on commit a3224bc

Please sign in to comment.