From f3d99e7bbe55e3dbfcdb682c1c2fe6f4e6a50b65 Mon Sep 17 00:00:00 2001 From: HttpRafa <60099368+HttpRafa@users.noreply.github.com> Date: Mon, 5 May 2025 16:29:23 +0200 Subject: [PATCH 1/2] fix: Cleanup senders before stopping the network stack --- controller/configs/wasm-plugins.toml | 2 +- controller/src/application.rs | 3 +++ controller/src/application/subscriber/manager.rs | 13 +++++++++++++ controller/src/application/subscriber/watcher.rs | 5 +++++ 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/controller/configs/wasm-plugins.toml b/controller/configs/wasm-plugins.toml index 52f04d66..cd81daa3 100644 --- a/controller/configs/wasm-plugins.toml +++ b/controller/configs/wasm-plugins.toml @@ -1,5 +1,5 @@ # This configuration is crucial for granting the plugins their required permissions -# https://httprafa.github.io/atomic-cloud/controller/plugins/wasm/permissions/ +# https://httprafa.github.io/atomic-cloud/plugins/wasm/permissions/ [[plugins]] name = "local" diff --git a/controller/src/application.rs b/controller/src/application.rs index 5faa3723..818b85b2 100644 --- a/controller/src/application.rs +++ b/controller/src/application.rs @@ -201,6 +201,9 @@ impl Controller { // Cleanup node manager self.nodes.cleanup().await?; + // Cleanup subscription manager + self.shared.subscribers.cleanup().await?; + // Cleanup screen manager self.shared.screens.cleanup().await?; diff --git a/controller/src/application/subscriber/manager.rs b/controller/src/application/subscriber/manager.rs index f5630913..1624c9ed 100644 --- a/controller/src/application/subscriber/manager.rs +++ b/controller/src/application/subscriber/manager.rs @@ -79,4 +79,17 @@ impl SubscriberManager { self.plugin.server_change_ready.cleanup().await; Ok(()) } + + pub async fn cleanup(&self) -> Result<()> { + // Drop all sender + self.network.channel.clear().await; + self.network.transfer.clear().await; + self.network.power.clear().await; + self.network.ready.clear().await; + + self.plugin.server_start.clear().await; + self.plugin.server_stop.clear().await; + self.plugin.server_change_ready.clear().await; + Ok(()) + } } diff --git a/controller/src/application/subscriber/watcher.rs b/controller/src/application/subscriber/watcher.rs index 8d9e07fb..ccbad0c9 100644 --- a/controller/src/application/subscriber/watcher.rs +++ b/controller/src/application/subscriber/watcher.rs @@ -57,4 +57,9 @@ impl Watcher { !subscribers.is_empty() }); } + + pub async fn clear(&self) { + self.0.write().await.clear(); + self.1.write().await.clear(); + } } From d45ca1d7b01a27560e38e2a56fd40270b4487dfb Mon Sep 17 00:00:00 2001 From: HttpRafa <60099368+HttpRafa@users.noreply.github.com> Date: Mon, 5 May 2025 17:34:09 +0200 Subject: [PATCH 2/2] feat: Add plugin permission checks --- controller/Cargo.toml | 2 +- controller/configs/wasm-plugins.toml | 23 +++----- .../src/application/plugin/runtime/wasm.rs | 2 + .../application/plugin/runtime/wasm/config.rs | 55 +++++++------------ .../plugin/runtime/wasm/ext/file.rs | 10 +++- .../plugin/runtime/wasm/ext/http.rs | 10 +++- .../plugin/runtime/wasm/ext/process.rs | 12 +++- .../application/plugin/runtime/wasm/init.rs | 24 ++++++-- plugins/cloudflare/configs/config.toml | 11 ++-- .../src/plugin/backend/batch/record.rs | 2 +- plugins/cloudflare/src/plugin/config.rs | 3 +- 11 files changed, 86 insertions(+), 68 deletions(-) diff --git a/controller/Cargo.toml b/controller/Cargo.toml index 1379daf2..8bc9de4d 100644 --- a/controller/Cargo.toml +++ b/controller/Cargo.toml @@ -15,7 +15,7 @@ anyhow = "1.0.98" # Getters and bitflags getset = "0.1.5" -bitflags = "2.9.0" +bitflags = { version = "2.9.0", features = ["serde"] } # Signal handling ctrlc = "3.4.6" diff --git a/controller/configs/wasm-plugins.toml b/controller/configs/wasm-plugins.toml index cd81daa3..cb799245 100644 --- a/controller/configs/wasm-plugins.toml +++ b/controller/configs/wasm-plugins.toml @@ -3,24 +3,15 @@ [[plugins]] name = "local" -inherit_stdio = false -inherit_args = false -inherit_env = false -inherit_network = false -allow_ip_name_lookup = false -allow_http = false -allow_process = true -allow_remove_dir_all = true +permissions = "ALLOW_PROCESS | ALLOW_REMOVE_DIR_ALL" mounts = [] [[plugins]] name = "pelican" -inherit_stdio = false -inherit_args = false -inherit_env = false -inherit_network = true -allow_ip_name_lookup = true -allow_http = true -allow_process = false -allow_remove_dir_all = false +permissions = "INHERIT_NETWORK | ALLOW_IP_NAME_LOOKUP | ALLOW_HTTP" +mounts = [] + +[[plugins]] +name = "cloudflare" +permissions = "INHERIT_NETWORK | ALLOW_IP_NAME_LOOKUP | ALLOW_HTTP" mounts = [] \ No newline at end of file diff --git a/controller/src/application/plugin/runtime/wasm.rs b/controller/src/application/plugin/runtime/wasm.rs index 3638fd4b..b34c7407 100644 --- a/controller/src/application/plugin/runtime/wasm.rs +++ b/controller/src/application/plugin/runtime/wasm.rs @@ -2,6 +2,7 @@ use std::sync::Arc; use anyhow::{anyhow, Result}; use common::error::FancyError; +use config::Permissions; use generated::{exports::plugin::system::bridge, plugin::system::data_types}; use listener::PluginListener; use node::PluginNode; @@ -51,6 +52,7 @@ pub(crate) struct PluginState { /* Plugin */ name: String, + permissions: Permissions, /* Wasmtime */ wasi: WasiCtx, diff --git a/controller/src/application/plugin/runtime/wasm/config.rs b/controller/src/application/plugin/runtime/wasm/config.rs index 1f596b98..6046a54a 100644 --- a/controller/src/application/plugin/runtime/wasm/config.rs +++ b/controller/src/application/plugin/runtime/wasm/config.rs @@ -1,6 +1,7 @@ use std::path::PathBuf; use anyhow::Result; +use bitflags::bitflags; use regex::Regex; use serde::{Deserialize, Serialize}; use simplelog::warn; @@ -22,49 +23,33 @@ pub struct PluginsConfig { plugins: Vec, } -#[allow( - clippy::struct_excessive_bools, - reason = "Mybe refactor this in the future to use bitflags" -)] +bitflags! { + #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] + #[serde(transparent)] + pub struct Permissions: u32 { + const INHERIT_STDIO = 1; + const INHERIT_ARGS = 1 << 1; + const INHERIT_ENV = 1 << 2; + const INHERIT_NETWORK = 1 << 3; + const ALLOW_IP_NAME_LOOKUP = 1 << 4; + const ALLOW_HTTP = 1 << 5; + const ALLOW_PROCESS = 1 << 6; + const ALLOW_REMOVE_DIR_ALL = 1 << 7; + const ALL = Self::INHERIT_STDIO.bits() | Self::INHERIT_ARGS.bits() | Self::INHERIT_ENV.bits() | Self::INHERIT_NETWORK.bits() | Self::ALLOW_IP_NAME_LOOKUP.bits() | Self::ALLOW_HTTP.bits() | Self::ALLOW_PROCESS.bits() | Self::ALLOW_REMOVE_DIR_ALL.bits(); + } +} + #[derive(Serialize, Deserialize)] pub struct PluginConfig { name: String, - inherit_stdio: bool, - inherit_args: bool, - inherit_env: bool, - inherit_network: bool, - allow_ip_name_lookup: bool, - allow_http: bool, - allow_process: bool, - allow_remove_dir_all: bool, + permissions: Permissions, mounts: Vec, } impl PluginConfig { - pub fn has_inherit_stdio(&self) -> bool { - self.inherit_stdio - } - pub fn has_inherit_args(&self) -> bool { - self.inherit_args - } - pub fn has_inherit_env(&self) -> bool { - self.inherit_env - } - pub fn has_inherit_network(&self) -> bool { - self.inherit_network - } - pub fn has_allow_ip_name_lookup(&self) -> bool { - self.allow_ip_name_lookup - } - pub fn _has_allow_http(&self) -> bool { - self.allow_http - } - pub fn _has_allow_process(&self) -> bool { - self.allow_process - } - pub fn _has_allow_remove_dir_all(&self) -> bool { - self.allow_remove_dir_all + pub fn get_permissions(&self) -> &Permissions { + &self.permissions } pub fn get_mounts(&self) -> &[Mount] { &self.mounts diff --git a/controller/src/application/plugin/runtime/wasm/ext/file.rs b/controller/src/application/plugin/runtime/wasm/ext/file.rs index 50e1b89e..bea7a52b 100644 --- a/controller/src/application/plugin/runtime/wasm/ext/file.rs +++ b/controller/src/application/plugin/runtime/wasm/ext/file.rs @@ -1,7 +1,8 @@ -use anyhow::Result; +use anyhow::{anyhow, Result}; use tokio::fs::remove_dir_all; use crate::application::plugin::runtime::wasm::{ + config::Permissions, generated::plugin::system::{ self, types::{Directory, ErrorMessage}, @@ -11,6 +12,13 @@ use crate::application::plugin::runtime::wasm::{ impl system::file::Host for PluginState { async fn remove_dir_all(&mut self, directory: Directory) -> Result> { + // Check if the plugin has permissions + if !self.permissions.contains(Permissions::ALLOW_REMOVE_DIR_ALL) { + return Err(anyhow!( + "Plugin tried to call remove_dir_all without the required permissions" + )); + } + Ok(remove_dir_all(Self::get_directory(&self.name, &directory)) .await .map_err(|error| format!("Failed to remove directory: {error}"))) diff --git a/controller/src/application/plugin/runtime/wasm/ext/http.rs b/controller/src/application/plugin/runtime/wasm/ext/http.rs index 0e13e280..f749c658 100644 --- a/controller/src/application/plugin/runtime/wasm/ext/http.rs +++ b/controller/src/application/plugin/runtime/wasm/ext/http.rs @@ -1,8 +1,9 @@ -use anyhow::Result; +use anyhow::{anyhow, Result}; use simplelog::warn; use tokio::task::spawn_blocking; use crate::application::plugin::runtime::wasm::{ + config::Permissions, generated::plugin::system::{ self, http::{Header, Method, Response}, @@ -19,6 +20,13 @@ impl system::http::Host for PluginState { headers: Vec
, body: Option>, ) -> Result> { + // Check if the plugin has permissions + if !self.permissions.contains(Permissions::ALLOW_HTTP) { + return Err(anyhow!( + "Plugin tried to send a http request without the required permissions" + )); + } + let name = self.name.clone(); Ok(spawn_blocking(move || { let mut request = match method { diff --git a/controller/src/application/plugin/runtime/wasm/ext/process.rs b/controller/src/application/plugin/runtime/wasm/ext/process.rs index 4fe9ba3b..11c57e3d 100644 --- a/controller/src/application/plugin/runtime/wasm/ext/process.rs +++ b/controller/src/application/plugin/runtime/wasm/ext/process.rs @@ -3,7 +3,7 @@ use std::{ process::{self, Stdio}, }; -use anyhow::Result; +use anyhow::{anyhow, Result}; use simplelog::debug; use tokio::{ io::{AsyncBufReadExt, AsyncWriteExt, BufReader, BufWriter}, @@ -15,6 +15,7 @@ use tokio::{ use wasmtime::component::Resource; use crate::application::plugin::runtime::wasm::{ + config::Permissions, generated::plugin::system::{ self, process::ExitStatus, @@ -26,7 +27,7 @@ use crate::application::plugin::runtime::wasm::{ #[cfg(unix)] use std::os::unix::process::ExitStatusExt; -const STREAM_BUFFER: usize = 64; +const STREAM_BUFFER: usize = 128; pub struct ProcessBuilder { command: String, @@ -139,6 +140,13 @@ impl system::process::HostProcessBuilder for PluginState { &mut self, instance: Resource, ) -> Result, ErrorMessage>> { + // Check if the plugin has permissions + if !self.permissions.contains(Permissions::ALLOW_PROCESS) { + return Err(anyhow!( + "Plugin tried to spawn a process without the required permissions" + )); + } + let builder = self.resources.get(&instance)?; debug!("Spawning process: {} {:?}", builder.command, builder.args); diff --git a/controller/src/application/plugin/runtime/wasm/init.rs b/controller/src/application/plugin/runtime/wasm/init.rs index 7e6d8b1a..bb770ff2 100644 --- a/controller/src/application/plugin/runtime/wasm/init.rs +++ b/controller/src/application/plugin/runtime/wasm/init.rs @@ -21,7 +21,7 @@ use crate::{ }; use super::{ - config::{verify_engine_config, PluginsConfig}, + config::{verify_engine_config, Permissions, PluginsConfig}, epoch::EpochInvoker, generated, Plugin, PluginState, }; @@ -195,20 +195,30 @@ impl Plugin { generated::Plugin::add_to_linker(&mut linker, |state: &mut PluginState| state)?; let mut wasi = WasiCtxBuilder::new(); + let mut permissions = Permissions::empty(); if let Some(config) = plugins_config.find_config(name) { - if config.has_inherit_stdio() { + if config + .get_permissions() + .contains(Permissions::INHERIT_STDIO) + { wasi.inherit_stdio(); } - if config.has_inherit_args() { + if config.get_permissions().contains(Permissions::INHERIT_ARGS) { wasi.inherit_args(); } - if config.has_inherit_env() { + if config.get_permissions().contains(Permissions::INHERIT_ENV) { wasi.inherit_env(); } - if config.has_inherit_network() { + if config + .get_permissions() + .contains(Permissions::INHERIT_NETWORK) + { wasi.inherit_network(); } - if config.has_allow_ip_name_lookup() { + if config + .get_permissions() + .contains(Permissions::ALLOW_IP_NAME_LOOKUP) + { wasi.allow_ip_name_lookup(true); } for mount in config.get_mounts() { @@ -219,6 +229,7 @@ impl Plugin { FilePerms::all(), )?; } + permissions = config.get_permissions().clone(); } let wasi = wasi .preopened_dir( @@ -237,6 +248,7 @@ impl Plugin { tasks, shared, name: name.to_string(), + permissions, wasi, resources, }, diff --git a/plugins/cloudflare/configs/config.toml b/plugins/cloudflare/configs/config.toml index 559d1b19..d55ec8eb 100644 --- a/plugins/cloudflare/configs/config.toml +++ b/plugins/cloudflare/configs/config.toml @@ -9,10 +9,13 @@ token = "" [[entries]] # The Zone ID from your Cloudflare dashboard zone = "" -# The DNS record name under which the plugin will create SRV records -name = "_minecraft._tcp.mynetwork.net" # A regular expression to identify servers that should receive an SRV record servers = "^lobby-.*" +# The DNS record name under which the plugin will create SRV records +name = "_minecraft._tcp.mynetwork.net" +# The TTL (Time to Live) of the DNS record +# NOTE: 1 = Auto / Otherwise in seconds +ttl = 60 # The priority assigned to the SRV records priority = 5 @@ -21,9 +24,9 @@ priority = 5 # NOTE: Adjust the values as necessary: https://www.geogebra.org/m/fmat9nc2 [entries.weight] # Scale factor: Multiplies the entire result to address resolution issues in scenarios with a low max value -a = 1.5 +a = 2 # Determines the rate of weight reduction; higher values result in a steeper decline # NOTE: Since weights can only be represented as integers, excessively high values may cause resolution issues. -k = 2.5 +k = 2 # Maximum number of players allowed per server max = 20 \ No newline at end of file diff --git a/plugins/cloudflare/src/plugin/backend/batch/record.rs b/plugins/cloudflare/src/plugin/backend/batch/record.rs index 522172e1..c26a79a9 100644 --- a/plugins/cloudflare/src/plugin/backend/batch/record.rs +++ b/plugins/cloudflare/src/plugin/backend/batch/record.rs @@ -42,7 +42,7 @@ impl BRecord { }, name: entry.name.clone(), proxied: false, - ttl: 1, + ttl: entry.ttl, r#type: "SRV".to_string(), id: record.id.clone(), }) diff --git a/plugins/cloudflare/src/plugin/config.rs b/plugins/cloudflare/src/plugin/config.rs index a98ac59a..8e6aeeae 100644 --- a/plugins/cloudflare/src/plugin/config.rs +++ b/plugins/cloudflare/src/plugin/config.rs @@ -27,8 +27,9 @@ pub struct Weight { #[derive(Deserialize, Clone)] pub struct Entry { pub zone: String, - pub name: String, pub servers: String, + pub name: String, + pub ttl: u16, pub priority: u16, pub weight: Weight, }