Implement validation for the invariants of the wireguard configuration: * All interfaces referenced in peer definitions must exist * Listen ports cannot be duplicated * Interface names must be unique on a node
Signed-off-by: Stefan Hanreich <[email protected]> --- proxmox-ve-config/Cargo.toml | 1 + proxmox-ve-config/src/sdn/fabric/mod.rs | 161 ++++++++++++++++-- .../src/sdn/fabric/section_config/node.rs | 2 +- .../section_config/protocol/wireguard.rs | 63 ++++++- 4 files changed, 210 insertions(+), 17 deletions(-) diff --git a/proxmox-ve-config/Cargo.toml b/proxmox-ve-config/Cargo.toml index fdcb331..bb1a057 100644 --- a/proxmox-ve-config/Cargo.toml +++ b/proxmox-ve-config/Cargo.toml @@ -33,3 +33,4 @@ frr = ["dep:proxmox-frr"] [dev-dependencies] insta = "1.21" +pretty_assertions = "1.4.0" diff --git a/proxmox-ve-config/src/sdn/fabric/mod.rs b/proxmox-ve-config/src/sdn/fabric/mod.rs index 53ce87f..7d1a2b9 100644 --- a/proxmox-ve-config/src/sdn/fabric/mod.rs +++ b/proxmox-ve-config/src/sdn/fabric/mod.rs @@ -31,7 +31,7 @@ use crate::sdn::fabric::section_config::protocol::ospf::{ }; use crate::sdn::fabric::section_config::protocol::wireguard::{ WireGuardDeletableProperties, WireGuardNode, WireGuardNodeDeletableProperties, - WireGuardNodeUpdater, WireGuardPropertiesUpdater, + WireGuardNodePeer, WireGuardNodeUpdater, WireGuardPropertiesUpdater, }; use crate::sdn::fabric::section_config::{FabricOrNode, Section}; @@ -73,6 +73,10 @@ pub enum FabricConfigError { OverlappingIp4Prefix(String, String, String, String), #[error("IPv6 prefix {0} in fabric '{1}' overlaps with IPv6 prefix {2} in fabric '{3}'")] OverlappingIp6Prefix(String, String, String, String), + #[error("peer configuration references non-existing interface")] + InvalidInterfaceReference, + #[error("WireGuard interface listen port duplicated in node configuration: {0}")] + DuplicatePort(String), } /// An entry in a [`FabricConfig`]. @@ -500,7 +504,49 @@ impl Validatable for FabricEntry { let mut ips = HashSet::new(); let mut ip6s = HashSet::new(); + if let FabricEntry::WireGuard(entry) = self { + // check if all interfaces referenced by the peer definitions exist inside the + // fabric + let mut all_interfaces = HashSet::new(); + let mut internal_peers = HashSet::new(); + + for node_id in entry.nodes.keys() { + let node_section = entry.node_section(node_id)?; + + if let WireGuardNode::Internal(node) = node_section.properties() { + all_interfaces.extend( + node.interfaces() + .map(|interface| (&node_section.id.node_id, &interface.name)), + ); + + internal_peers.extend(node.peers().filter_map(|peer| { + if let WireGuardNodePeer::Internal(peer) = peer { + return Some((&peer.node, &peer.node_iface)); + } + + None + })); + } + } + + if !(all_interfaces.is_superset(&internal_peers)) { + return Err(FabricConfigError::InvalidInterfaceReference); + } + } + for (_id, node) in self.nodes() { + node.validate()?; + + // Node IPs need to be unique inside a fabric + if !node.ip().map(|ip| ips.insert(ip)).unwrap_or(true) { + return Err(FabricConfigError::DuplicateNodeIp(fabric.id().to_string())); + } + + // Node IPs need to be unique inside a fabric + if !node.ip6().map(|ip| ip6s.insert(ip)).unwrap_or(true) { + return Err(FabricConfigError::DuplicateNodeIp(fabric.id().to_string())); + } + // Check IPv4 prefix and ip match (fabric.ip_prefix(), node.ip()) { (None, Some(ip)) => { @@ -554,18 +600,6 @@ impl Validatable for FabricEntry { } _ => {} } - - // Node IPs need to be unique inside a fabric - if !node.ip().map(|ip| ips.insert(ip)).unwrap_or(true) { - return Err(FabricConfigError::DuplicateNodeIp(fabric.id().to_string())); - } - - // Node IPs need to be unique inside a fabric - if !node.ip6().map(|ip| ip6s.insert(ip)).unwrap_or(true) { - return Err(FabricConfigError::DuplicateNodeIp(fabric.id().to_string())); - } - - node.validate()?; } fabric.validate() @@ -634,6 +668,7 @@ impl Validatable for FabricConfig { } // validate that each (node, interface) combination exists only once across all fabrics + // additionally, for wireguard check the listen ports of the interfaces as well for entry in self.fabrics.values() { if let FabricEntry::Ospf(entry) = entry { if !ospf_area.insert( @@ -662,8 +697,14 @@ impl Validatable for FabricConfig { return Err(FabricConfigError::DuplicateInterface); } } - Node::WireGuard(_node_section) => { - return Ok(()); + Node::WireGuard(node_section) => { + if let WireGuardNode::Internal(internal_node) = node_section.properties() { + if !internal_node.interfaces().all(|interface| { + node_interfaces.insert((node_id, interface.name.as_str())) + }) { + return Err(FabricConfigError::DuplicateInterface); + } + } } } } @@ -952,3 +993,93 @@ impl Valid<FabricConfig> { Section::write_section_config("fabrics.cfg", &self.into_section_config()) } } + +#[cfg(test)] +mod tests { + use crate::sdn::fabric::FabricConfig; + use proxmox_section_config::typed::ApiSectionDataEntry; + + use super::*; + + #[test] + fn test_wireguard_validation_duplicate_interface() -> Result<(), anyhow::Error> { + let section_config = r#" +wireguard_fabric: wireg + +wireguard_node: wireg_internal + role internal + endpoint 192.0.2.1:123 + public_key Kay64UG8yvCyLhqU000LxzYeUm0L/hLIl5S8kyKWbdc= + interfaces name=wg0,listen_port=51111,public_key=Kay64UG8yvCyLhqU000LxzYeUm0L/hLIl5S8kyKWbdc= + interfaces name=wg0,listen_port=51112,public_key=Kay64UG8yvCyLhqU000LxzYeUm0L/hLIl5S8kyKWbdc= +"#; + let parsed_config = Section::parse_section_config("fabrics.cfg", section_config)?; + FabricConfig::from_section_config(parsed_config) + .expect_err("duplicate interface name on node"); + + Ok(()) + } + + #[test] + fn test_wireguard_validation_duplicate_listen_port() -> Result<(), anyhow::Error> { + let section_config = r#" +wireguard_fabric: wireg + +wireguard_node: wireg_internal + role internal + endpoint 192.0.2.1:123 + public_key Kay64UG8yvCyLhqU000LxzYeUm0L/hLIl5S8kyKWbdc= + interfaces name=wg0,listen_port=51111,public_key=Kay64UG8yvCyLhqU000LxzYeUm0L/hLIl5S8kyKWbdc= + interfaces name=wg1,listen_port=51111,public_key=Kay64UG8yvCyLhqU000LxzYeUm0L/hLIl5S8kyKWbdc= +"#; + let parsed_config = Section::parse_section_config("fabrics.cfg", section_config)?; + FabricConfig::from_section_config(parsed_config) + .expect_err("duplicate listen_port on node"); + + Ok(()) + } + + #[test] + fn test_wireguard_validation_node_interface_does_not_exist() -> Result<(), anyhow::Error> { + let section_config = r#" +wireguard_fabric: wireg + +wireguard_node: wireg_internal + role internal + endpoint 192.0.2.1:123 + public_key Kay64UG8yvCyLhqU000LxzYeUm0L/hLIl5S8kyKWbdc= + interfaces name=wg0,listen_port=51111,public_key=Kay64UG8yvCyLhqU000LxzYeUm0L/hLIl5S8kyKWbdc= + peers type=internal,node=invalid,node_iface=invalid,iface=wg0 +"#; + let parsed_config = Section::parse_section_config("fabrics.cfg", section_config)?; + FabricConfig::from_section_config(parsed_config) + .expect_err("interface referenced in peer definition does not exist"); + + Ok(()) + } + + #[test] + fn test_wireguard_validation_local_interface_does_not_exist() -> Result<(), anyhow::Error> { + let section_config = r#" +wireguard_fabric: wireg + +wireguard_node: wireg_internal + role internal + endpoint 192.0.2.1:123 + public_key Kay64UG8yvCyLhqU000LxzYeUm0L/hLIl5S8kyKWbdc= + interfaces name=wg0,listen_port=51111,public_key=Kay64UG8yvCyLhqU000LxzYeUm0L/hLIl5S8kyKWbdc= + +wireguard_node: wireg_internal2 + role internal + endpoint 192.0.2.2:123 + public_key Kay64UG8yvCyLhqU000LxzYeUm0L/hLIl5S8kyKWbdc= + interfaces name=wg0,listen_port=51111,public_key=Kay64UG8yvCyLhqU000LxzYeUm0L/hLIl5S8kyKWbdc= + peers type=internal,node=internal,node_iface=wg0,iface=wg1 +"#; + let parsed_config = Section::parse_section_config("fabrics.cfg", section_config)?; + FabricConfig::from_section_config(parsed_config) + .expect_err("local interface in peer definition does not exist"); + + Ok(()) + } +} diff --git a/proxmox-ve-config/src/sdn/fabric/section_config/node.rs b/proxmox-ve-config/src/sdn/fabric/section_config/node.rs index 77ce15f..c7b26f4 100644 --- a/proxmox-ve-config/src/sdn/fabric/section_config/node.rs +++ b/proxmox-ve-config/src/sdn/fabric/section_config/node.rs @@ -239,7 +239,7 @@ impl Validatable for Node { match self { Node::Openfabric(node_section) => node_section.validate(), Node::Ospf(node_section) => node_section.validate(), - Node::WireGuard(_node_section) => Ok(()), + Node::WireGuard(node_section) => node_section.validate(), } } } diff --git a/proxmox-ve-config/src/sdn/fabric/section_config/protocol/wireguard.rs b/proxmox-ve-config/src/sdn/fabric/section_config/protocol/wireguard.rs index 3765b89..3acd856 100644 --- a/proxmox-ve-config/src/sdn/fabric/section_config/protocol/wireguard.rs +++ b/proxmox-ve-config/src/sdn/fabric/section_config/protocol/wireguard.rs @@ -28,6 +28,7 @@ //! definition can be overridden in the peer definition, if e.g. a different endpoint is required //! for connecting to a node. +use std::collections::HashSet; use std::ops::{Deref, DerefMut}; use anyhow::Result; @@ -44,7 +45,10 @@ use proxmox_sdn_types::wireguard::PersistentKeepalive; use proxmox_wireguard::PublicKey; use serde::{Deserialize, Serialize}; -use crate::sdn::fabric::section_config::node::NodeId; +use crate::common::valid::Validatable; +use crate::sdn::fabric::section_config::fabric::FabricSection; +use crate::sdn::fabric::section_config::node::{NodeId, NodeSection}; +use crate::sdn::fabric::FabricConfigError; pub const WIREGUARD_INTERFACE_NAME_REGEX_STR: &str = "[a-zA-Z0-9][a-zA-Z0-9-]{0,6}[a-zA-Z0-9]?"; @@ -79,6 +83,14 @@ pub struct WireGuardProperties { pub(crate) persistent_keepalive: Option<PersistentKeepalive>, } +impl Validatable for FabricSection<WireGuardProperties> { + type Error = FabricConfigError; + + fn validate(&self) -> Result<(), Self::Error> { + Ok(()) + } +} + #[derive(Clone, Debug, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum WireGuardDeletableProperties { @@ -159,6 +171,18 @@ impl ApiType for WireGuardNode { .schema(); } +impl Validatable for NodeSection<WireGuardNode> { + type Error = FabricConfigError; + + fn validate(&self) -> Result<(), Self::Error> { + if let WireGuardNode::Internal(node) = self.properties() { + return node.validate(); + } + + Ok(()) + } +} + #[derive(Debug, Clone, Serialize, Deserialize, Hash)] #[serde(rename_all = "snake_case", tag = "role")] pub enum WireGuardNodeUpdater { @@ -291,6 +315,43 @@ impl InternalWireGuardNode { } } +impl Validatable for InternalWireGuardNode { + type Error = FabricConfigError; + + /// Validates the [FabricSection<WireGuardNodeProperties>]. + /// + /// Checks if we have either an IPv4 or an IPv6 address. If neither is set, return an error. + fn validate(&self) -> Result<(), Self::Error> { + let mut local_interfaces = HashSet::new(); + let mut listen_ports = HashSet::new(); + + for interface in self.interfaces() { + // check if interface names are unique + if !local_interfaces.insert(&interface.name) { + return Err(FabricConfigError::DuplicateInterface); + } + + // check if listen ports are unique + if !listen_ports.insert(interface.listen_port) { + return Err(FabricConfigError::DuplicatePort( + interface.listen_port.to_string(), + )); + } + } + + for peer in self.peers() { + if let WireGuardNodePeer::Internal(peer) = peer { + // check if referenced local interface exists + if !local_interfaces.contains(&peer.iface) { + return Err(FabricConfigError::InvalidInterfaceReference); + } + } + } + + Ok(()) + } +} + #[api( properties: { allowed_ips: { -- 2.47.3
