diff --git a/API_CHANGELOG.md b/API_CHANGELOG.md index 6bae5292..83524345 100644 --- a/API_CHANGELOG.md +++ b/API_CHANGELOG.md @@ -58,6 +58,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - **The route server now routes its tunnel pool's own blocks.** An address on a point-to-point interface does not route the rest of its prefix, so a route server holding `10.66.0.1/16` answered "network is unreachable" for every node in that pool — the peers were configured correctly and unreachable. Found by an end-to-end harness that builds both ends of a tunnel and sends real packets, not by review. +- **A node's status now reports the packet filter around its guests** — `GET /status` on the node control API gains `dataplane.firewall`: whether the machine has a working nftables, whether LNVPS's ruleset is loaded, whether guests are isolated from each other at layer 2, how many guest bindings are enforced, which ruleset the *kernel* is running (a tag read back out of a rule comment, not remembered by the daemon), and how many packets have been dropped for claiming an address the guest was not assigned. That last number is the only one here that says something about a customer rather than a node: a guest that is spoofing is either compromised or hostile, and LNVPS would rather learn it from a counter than from an upstream abuse report. A node whose filter is not loaded now reports itself **unhealthy** — an unfiltered node is one where any guest can be any other, which is worse than a node carrying nobody. + - **A node can fetch its whole data plane in one document** — `GET /api/v1/node/dataplane` (node token) returns the tunnel it already gets from `/node/tunnel`, plus the gateway addresses the node must answer for, and the guests assigned to it (address, gateway, MAC). One call rather than three because the node applies these together or not at all: a bridge with no tunnel carries nothing, a tunnel with no guest routes carries nothing back, and a document that can be half-fetched is a data plane that can be half-applied. The guest list is also the anti-spoof list — an address not in it is not that node's to send from. The gateway is the one the guest was actually configured with, taken from its IP range: it belongs to the range rather than to the node, and the guest believes it is on-link, so the node has to answer for it rather than invent one. - **A node takes one address, not a point-to-point link** — `address4`/`address6` in the tunnel response are now a `/32` and a `/128`, and `gateway4`/`gateway6` are one address shared by every node on the pool rather than a per-node link address. WireGuard is layer 3 and point-to-point: the node needs no gateway on its own side (`ip route add default dev wg0` suffices), so a `/31` spent two addresses describing something that needs one — and forced the route server to carry one address per node on a single interface, thousands of them on a /16 pool. Pool capacity is reported accordingly: a /24 places 253 nodes (256 less the block's network address, the route server's address after it, and the broadcast address) where it previously reported 128 links. diff --git a/Cargo.lock b/Cargo.lock index 5efc91dd..bf6da725 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -236,7 +236,7 @@ checksum = "965c2d33e53cb6b267e148a4cb0760bc01f4904c1cd4bb4002a085bb016d1490" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", "synstructure", ] @@ -248,7 +248,7 @@ checksum = "7b18050c2cd6fe86c3a76584ef5e0baf286d038cda203eb6223df2cc413565f7" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -376,7 +376,7 @@ checksum = "81872a8e595e8ceceab71c6ba1f9078e313b452a1e31934e6763ef5d308705e4" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -398,7 +398,7 @@ checksum = "c7c24de15d275a1ecfd47a380fb4d5ec9bfe0933f309ed5e705b775596a3574d" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -409,7 +409,7 @@ checksum = "9035ad2d096bed7955a320ee7e2230574d28fd3c3a0f186cbea1ff3c7eed5dbb" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -557,7 +557,7 @@ checksum = "7aa268c23bfbbd2c4363b9cd302a4f504fb2a9dfe7e3451d66f35dd392e20aca" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -973,7 +973,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -1356,7 +1356,7 @@ checksum = "f46882e17999c6cc590af592290432be3bce0428cb0d5f8b6715e4dc7b383eb3" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -1380,7 +1380,7 @@ dependencies = [ "proc-macro2", "quote", "strsim", - "syn", + "syn 2.0.118", ] [[package]] @@ -1391,7 +1391,7 @@ checksum = "fc34b93ccb385b40dc71c6fceac4b2ad23662c7eeb248cf10d529b7e055b6ead" dependencies = [ "darling_core", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -1489,7 +1489,7 @@ checksum = "780eb241654bf097afb00fc5f054a09b687dad862e485fdcf8399bb056565370" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -1552,7 +1552,7 @@ dependencies = [ "darling", "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -1562,7 +1562,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ab63b0e2bf4d5928aff72e83a7dace85d7bba5fe12dcc3c5a572d78caffd3f3c" dependencies = [ "derive_builder_core", - "syn", + "syn 2.0.118", ] [[package]] @@ -1583,7 +1583,7 @@ dependencies = [ "proc-macro2", "quote", "rustc_version", - "syn", + "syn 2.0.118", ] [[package]] @@ -1627,7 +1627,7 @@ checksum = "1ac70aa55017e108007fbaf5aa0f54b021c98f92ff8af59d42eda9da96e3dd4f" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -1721,7 +1721,7 @@ dependencies = [ "enum-ordinalize", "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -1820,7 +1820,7 @@ checksum = "8ca9601fb2d62598ee17836250842873a413586e5d7ed88b356e38ddbb0ec631" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -1832,7 +1832,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -2149,7 +2149,7 @@ checksum = "2d6d3cde68c518367be28956066ddfef33813991b77a55005a69dae04bf3b10b" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -3069,7 +3069,7 @@ checksum = "782d32378dddf207193ac91cefb848ad41abb58195c95168e1291227a0832b47" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -3099,7 +3099,7 @@ dependencies = [ "quote", "rustc_version", "simd_cesu8", - "syn", + "syn 2.0.118", ] [[package]] @@ -3118,7 +3118,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "38c0b942f458fe50cdac086d2f946512305e5631e720728f2a61aabcd47a6264" dependencies = [ "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -3282,7 +3282,7 @@ dependencies = [ "http", "json-patch", "k8s-openapi", - "schemars", + "schemars 0.8.22", "serde", "serde-value", "serde_json", @@ -3300,7 +3300,7 @@ dependencies = [ "quote", "serde", "serde_json", - "syn", + "syn 2.0.118", ] [[package]] @@ -3736,6 +3736,7 @@ dependencies = [ "lnvps_host_util", "log 0.4.32", "netlink-packet-route 0.30.0", + "nftables", "nix 0.30.1", "nostr 0.44.3", "rand 0.9.4", @@ -4095,6 +4096,21 @@ dependencies = [ "tokio", ] +[[package]] +name = "nftables" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c57e7343eed9e9330e084eef12651b15be3c8ed7825915a0ffa33736b852bed" +dependencies = [ + "schemars 1.2.2", + "serde", + "serde_json", + "serde_path_to_error", + "strum", + "strum_macros", + "thiserror 2.0.18", +] + [[package]] name = "nix" version = "0.30.1" @@ -4362,7 +4378,7 @@ dependencies = [ "quote", "serde", "serde_json", - "syn", + "syn 2.0.118", "walkdir", ] @@ -4399,7 +4415,7 @@ checksum = "a948666b637a0f465e8564c73e89d4dde00d72d4d473cc972f390fc3dcee7d9c" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -4720,7 +4736,7 @@ dependencies = [ "pest_meta", "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -4770,7 +4786,7 @@ checksum = "c96395f0a926bc13b1c17622aaddda1ecb55d49c8f1bf9777e4d877800a43f8b" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -4959,7 +4975,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "479ca8adacdd7ce8f1fb39ce9ecccbfe93a3f1344b3d0d97f20bc0196208f62b" dependencies = [ "proc-macro2", - "syn", + "syn 2.0.118", ] [[package]] @@ -5048,7 +5064,7 @@ dependencies = [ "prost", "prost-types", "regex", - "syn", + "syn 2.0.118", "tempfile", ] @@ -5062,7 +5078,7 @@ dependencies = [ "itertools", "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -5340,6 +5356,26 @@ dependencies = [ "bitflags", ] +[[package]] +name = "ref-cast" +version = "1.0.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "216e8f773d7923bcba9ceb86a86c93cabb3903a11872fc3f138c49630e50b96d" +dependencies = [ + "ref-cast-impl", +] + +[[package]] +name = "ref-cast-impl" +version = "1.0.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2c9283685feec7d69af75fb0e858d5e7378f33fe4fc699383b2916ab9273e03c" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.3", +] + [[package]] name = "regex" version = "1.12.4" @@ -5914,7 +5950,20 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3fbf2ae1b8bc8e02df939598064d22402220cd5bbcca1c76f7d6a310974d5615" dependencies = [ "dyn-clone", - "schemars_derive", + "schemars_derive 0.8.22", + "serde", + "serde_json", +] + +[[package]] +name = "schemars" +version = "1.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "687274d293b6cdc6e73e0fee520bf2049650090d7164f87672d212a3c530cf4a" +dependencies = [ + "dyn-clone", + "ref-cast", + "schemars_derive 1.2.2", "serde", "serde_json", ] @@ -5927,8 +5976,20 @@ checksum = "32e265784ad618884abaea0600a9adf15393368d840e0222d101a072f3f7534d" dependencies = [ "proc-macro2", "quote", - "serde_derive_internals", - "syn", + "serde_derive_internals 0.29.1", + "syn 2.0.118", +] + +[[package]] +name = "schemars_derive" +version = "1.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d98c67716b46af2f0b8cf752abc930f6f9aecfbf671ecfb531db8a31dbe4e2ba" +dependencies = [ + "proc-macro2", + "quote", + "serde_derive_internals 0.30.0", + "syn 3.0.3", ] [[package]] @@ -6137,7 +6198,7 @@ checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -6148,7 +6209,18 @@ checksum = "18d26a20a969b9e3fdf2fc2d9f21eda6c40e2de84c9408bb5d3b05d499aae711" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", +] + +[[package]] +name = "serde_derive_internals" +version = "0.30.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f852137cce035d6a4df67ccce505ff6b3e9fd3a10e3e52b24dc71e650bb1a9bd" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.3", ] [[package]] @@ -6502,7 +6574,7 @@ dependencies = [ "quote", "sqlx-core", "sqlx-macros-core", - "syn", + "syn 2.0.118", ] [[package]] @@ -6525,7 +6597,7 @@ dependencies = [ "sqlx-mysql", "sqlx-postgres", "sqlx-sqlite", - "syn", + "syn 2.0.118", "tokio", "url", ] @@ -6771,6 +6843,24 @@ version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" +[[package]] +name = "strum" +version = "0.27.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af23d6f6c1a224baef9d3f61e287d2761385a5b88fdab4eb4c6f11aeb54c4bcf" + +[[package]] +name = "strum_macros" +version = "0.27.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7695ce3845ea4b33927c055a39dc438a45b059f7c1b3d91d38d10355fb8cbca7" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn 2.0.118", +] + [[package]] name = "subtle" version = "2.6.1" @@ -6788,6 +6878,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "syn" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "sync_wrapper" version = "1.0.2" @@ -6805,7 +6906,7 @@ checksum = "728a70f3dbaf5bab7f0c4b1ac8d7ae5ea60a4b5549c8a5914361c99147a709d2" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -6874,7 +6975,7 @@ checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -6885,7 +6986,7 @@ checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -6976,7 +7077,7 @@ checksum = "385a6cb71ab9ab790c5fe8d67f1645e6c450a7ce006a33de03daa956cf70a496" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -7152,7 +7253,7 @@ dependencies = [ "prost-build", "prost-types", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -7228,7 +7329,7 @@ checksum = "7490cfa5ec963746568740651ac6781f701c9c5ea257c58e057f3ba8cf69e8da" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -7566,7 +7667,7 @@ dependencies = [ "bumpalo", "proc-macro2", "quote", - "syn", + "syn 2.0.118", "wasm-bindgen-shared", ] @@ -7804,7 +7905,7 @@ checksum = "053e2e040ab57b9dc951b72c264860db7eb3b0200ba345b4e4c3b14f67855ddf" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -7815,7 +7916,7 @@ checksum = "3f316c4a2570ba26bbec722032c4099d8c8bc095efccdc15688708623367e358" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -8258,7 +8359,7 @@ checksum = "de844c262c8848816172cef550288e7dc6c7b7814b4ee56b3e1553f275f1858e" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", "synstructure", ] @@ -8279,7 +8380,7 @@ checksum = "1ae7f38b72ec2a254e2b87ef277cf2cd4fb97cbebf944faa6f33354da0867930" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -8299,7 +8400,7 @@ checksum = "11532158c46691caf0f2593ea8358fed6bbf68a0315e80aae9bd41fbade684a1" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", "synstructure", ] @@ -8320,7 +8421,7 @@ checksum = "3c50655cbb0fe3fc43170059e702f1ce5e19b84cec58dc87b037a09935c2f328" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] @@ -8353,7 +8454,7 @@ checksum = "625dc425cab0dca6dc3c3319506e6593dcb08a9f387ea3b284dbd52a92c40555" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.118", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 63af264e..3198a472 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,6 +43,7 @@ tower-http = { version = "0.6", features = [ ] } config = { version = "0.15", features = ["yaml"] } hex = "0.4" +nftables = "0.6" ipnetwork = "0.21" payments-rs = { version = "0.7.0", default-features = false } async-trait = "0.1" diff --git a/docs/agents/e2e-tests.md b/docs/agents/e2e-tests.md index f0e6ece1..fcb17e39 100644 --- a/docs/agents/e2e-tests.md +++ b/docs/agents/e2e-tests.md @@ -359,7 +359,15 @@ that proves a packet moves. This harness caught four things nothing else did: in the pool. Not visible in any unit test, because the code did exactly what it was written to do. +The same harness covers the packet filter (`lnvps_node::fw`), which is where its rules are +proved to load at all — nftables rejects a ruleset the kernel cannot express, and no unit test +against typed objects can tell you whether `nft` will accept them: a guest is given a second address +nobody assigned it — which any customer with root in their own VM can do — and the node's spoof +counter is asserted to move. The counter, not the ping's exit status: a spoofed packet gets no +reply in any case, for want of a return route. + Coverage note: the netlink implementation (`lnvps_node::net::kernel`) and -`lnvps_node::netns` are exercised here rather than by the normal test run, the +`lnvps_node::netns`, along with `lnvps_node::fw`'s `SystemFirewall`, are exercised here rather +than by the normal test run, the same way `lnvps_fw`'s datapath is covered by its netns harness. Measure them with `sudo -E cargo llvm-cov -p lnvps_node -- --include-ignored`. diff --git a/lnvps_e2e/tests/tunnel_netns.rs b/lnvps_e2e/tests/tunnel_netns.rs index e3a91c72..63b6a7f9 100644 --- a/lnvps_e2e/tests/tunnel_netns.rs +++ b/lnvps_e2e/tests/tunnel_netns.rs @@ -40,6 +40,11 @@ const RS_INNER: &str = "10.66.0.1/24"; const NODE_INNER: &str = "10.66.0.2/32"; /// A customer address, as LNVPS assigns it to a guest on this node. const GUEST_ADDRESS: &str = "203.0.113.5"; +/// The guest's MAC, which the filter binds its address to. LNVPS knows it +/// because LNVPS assigned it when the VM was created. +const GUEST_MAC: &str = "52:54:00:e2:e0:05"; +/// An address nobody assigned this guest, used to prove the filter drops it. +const SPOOFED_ADDRESS: &str = "203.0.113.99"; const GUEST_GATEWAY: &str = "203.0.113.1"; /// Namespaces, torn down on drop even when a test panics. @@ -262,10 +267,11 @@ async fn a_guest_behind_a_node_is_reachable_from_the_route_server() -> Result<() guests: vec![DesiredGuest { address: format!("{GUEST_ADDRESS}/32"), gateway: GUEST_GATEWAY.to_string(), - mac: None, + mac: Some(GUEST_MAC.to_string()), }], }; - lnvps_node::net::apply(&kernel, &desired, &node_key).await?; + let firewall = lnvps_node::fw::SystemFirewall::new(topology.open_dataplane()?); + lnvps_node::net::apply(&kernel, &firewall, &desired, &node_key).await?; // ---- a guest on the node's bridge, addressed as a customer's VM is run( @@ -291,6 +297,7 @@ async fn a_guest_behind_a_node_is_reachable_from_the_route_server() -> Result<() "ip", &["link", "set", "e2e-guest", "netns", &topology.guest], )?; + topology.in_guest(&["ip", "link", "set", "e2e-guest", "address", GUEST_MAC])?; topology.in_guest(&[ "ip", "addr", @@ -299,6 +306,17 @@ async fn a_guest_behind_a_node_is_reachable_from_the_route_server() -> Result<() "dev", "e2e-guest", ])?; + // A second address the guest simply gave itself. Nothing stops a customer + // doing this — root in their own VM is the whole product — so the filter is + // what has to stop the packets. + topology.in_guest(&[ + "ip", + "addr", + "add", + &format!("{SPOOFED_ADDRESS}/24"), + "dev", + "e2e-guest", + ])?; topology.in_guest(&["ip", "link", "set", "e2e-guest", "up"])?; // The guest is configured with its range's gateway and believes it is // on-link — which is exactly why the node holds that address and answers @@ -328,7 +346,7 @@ async fn a_guest_behind_a_node_is_reachable_from_the_route_server() -> Result<() // The node reports itself healthy only once that has happened: WireGuard comes // up perfectly happily with a peer that never answers. - let state = lnvps_node::net::observe(&kernel).await?; + let state = lnvps_node::net::observe(&kernel, &firewall).await?; assert!(state.tunnel_up, "{state:?}"); assert!(state.bridge_up, "{state:?}"); assert!( @@ -336,6 +354,61 @@ async fn a_guest_behind_a_node_is_reachable_from_the_route_server() -> Result<() "a tunnel that carried packets reported no handshake: {state:?}" ); assert!(state.healthy(), "{state:?}"); + + // ---- and what the filter is for + // + // The guest's own address reaches the route server; the address it made up + // does not. This is the check the route server's AllowedIPs cannot make: + // both addresses are inside the node's peer, so from the far end a guest + // stealing its neighbour's address is indistinguishable from the real + // thing. + let rs_inner = "10.66.0.1"; + topology + .in_guest(&["ping", "-c", "2", "-W", "5", "-I", GUEST_ADDRESS, rs_inner]) + .context("a guest could not reach the route server from its own address")?; + assert!( + topology + .in_guest(&[ + "ping", + "-c", + "2", + "-W", + "3", + "-I", + SPOOFED_ADDRESS, + rs_inner + ]) + .is_err(), + "a guest reached the network sourcing an address LNVPS never assigned it:\n{}", + topology + .in_dataplane(&["nft", "list", "ruleset"]) + .unwrap_or_default() + ); + + // ...and the filter says which ruleset it is enforcing, read back off the + // kernel rather than remembered by the daemon. + let firewall_state = lnvps_node::fw::observe(&firewall).await; + assert!(firewall_state.available, "{firewall_state:?}"); + assert!(firewall_state.present, "{firewall_state:?}"); + assert!( + firewall_state.isolated, + "guests were not isolated from each other at layer 2: {firewall_state:?}" + ); + assert_eq!( + firewall_state.ruleset, + Some(lnvps_node::fw::fingerprint( + &lnvps_node::fw::Policy::from_desired(&desired)? + )), + "the machine is enforcing a different ruleset from the one applied" + ); + + // A second apply changes nothing: this runs every few seconds forever, and + // a node that reported a change on every poll would make the log useless. + let again = lnvps_node::net::apply(&kernel, &firewall, &desired, &node_key).await?; + assert!( + !again.iter().any(|c| c.contains("nft")), + "the filter was reloaded when nothing had changed: {again:?}" + ); Ok(()) } @@ -361,8 +434,10 @@ async fn the_operators_machine_keeps_its_own_network() -> Result<()> { run("ip", &["link", "add", "wg0", "type", "wireguard"])?; let kernel = lnvps_node::net::Kernel::in_namespace(topology.open_dataplane()?)?; + let firewall = lnvps_node::fw::SystemFirewall::new(topology.open_dataplane()?); lnvps_node::net::apply( &kernel, + &firewall, &DesiredDataPlane { tunnel: DesiredTunnel { address4: Some(NODE_INNER.to_string()), diff --git a/lnvps_node/Cargo.toml b/lnvps_node/Cargo.toml index 428d8c67..09bb9c1e 100644 --- a/lnvps_node/Cargo.toml +++ b/lnvps_node/Cargo.toml @@ -22,6 +22,7 @@ nostr = { version = "0.44", default-features = false, features = ["std"] } x25519-dalek = { version = "3.0.0", features = ["static_secrets"] } rand = "0.9" hex.workspace = true +nftables.workspace = true # Outbound calls to LNVPS: the node fetches the data plane it should be running. reqwest = { version = "0.12", default-features = false, features = ["rustls-tls", "json"] } # Self-signed TLS identity, pinned by LNVPS at registration. Same crate and diff --git a/lnvps_node/src/control.rs b/lnvps_node/src/control.rs index f80fa1a9..79fe0738 100644 --- a/lnvps_node/src/control.rs +++ b/lnvps_node/src/control.rs @@ -60,6 +60,8 @@ pub struct ControlState { /// How the node reads its own network back. Held here so a test can supply /// a machine that is not the one the tests run on. pub net: Arc, + /// How the node reads its packet filter back, for the same reason. + pub fw: Arc, } impl ControlState { @@ -68,18 +70,20 @@ impl ControlState { control_pubkey: PublicKey, addr: SocketAddr, net: Arc, + fw: Arc, ) -> Self { Self { control_pubkey, replay: Mutex::new(ReplayGuard::new(REPLAY_WINDOW, REPLAY_CAPACITY)), base_url: format!("https://{addr}"), net, + fw, } } /// The data plane as this machine actually has it. pub async fn observe(&self) -> crate::net::DataPlaneState { - crate::net::observe(self.net.as_ref()) + crate::net::observe(self.net.as_ref(), self.fw.as_ref()) .await .unwrap_or_default() } @@ -250,6 +254,7 @@ mod tests { keys.public_key(), addr.parse().unwrap(), Arc::new(crate::net::tests::FakeKernel::default()), + Arc::new(crate::fw::tests::FakeFirewall::default()), )) } @@ -309,6 +314,7 @@ mod tests { keys.public_key(), addr, Arc::new(crate::net::tests::FakeKernel::default()), + Arc::new(crate::fw::tests::FakeFirewall::default()), ); let url = format!("https://{ADDR}/api/v1/status"); diff --git a/lnvps_node/src/credential.rs b/lnvps_node/src/credential.rs index e7cb2508..c9e83065 100644 --- a/lnvps_node/src/credential.rs +++ b/lnvps_node/src/credential.rs @@ -15,7 +15,6 @@ use std::fs; use std::path::{Path, PathBuf}; use anyhow::{Context, Result, bail}; -use base64::Engine; use serde::{Deserialize, Serialize}; /// Where the node's token lives. diff --git a/lnvps_node/src/fw.rs b/lnvps_node/src/fw.rs new file mode 100644 index 00000000..8b4aa2b3 --- /dev/null +++ b/lnvps_node/src/fw.rs @@ -0,0 +1,848 @@ +//! The packet filter around a node's guests. +//! +//! 4c1 put the data plane in its own network namespace, which settled a whole +//! class of rules by construction: there is no interface from `br-lnvps` to the +//! operator's LAN, so there is nothing to write a rule against. What is left is +//! what no topology can express. +//! +//! - **A guest may only source the addresses LNVPS assigned it.** The route +//! server's `AllowedIPs` already stops one node claiming another's addresses, +//! but it cannot see *inside* a node: both guests' addresses legitimately +//! belong to that node's peer, so guest A pretending to be guest B is +//! invisible from the far end and has to be caught here. +//! - **Guests may not talk to each other at layer 2.** They share a bridge, and +//! proxy ARP tells each of them that every address is on-link, so without +//! this a tenant can ARP-poison or ND-poison their neighbours — an attack +//! that never reaches the IP layer the rest of this module filters at. +//! Dropping the bridge's forward hook does not disconnect them: it forces +//! their traffic to be *routed* by the node, which is where it can be +//! checked, and is exactly what they would get if they were on two different +//! nodes. +//! - **TCP MSS is clamped to the path MTU**, because a guest that ignores path +//! MTU discovery otherwise gets a connection that opens and then hangs. +//! +//! The ruleset is owned wholesale and replaced in one transaction. The daemon +//! never appends to an operator's chains: it builds a complete table and swaps +//! it, so there is no moment when a guest is running unfiltered and no way for +//! a half-applied ruleset to survive a crash. +//! +//! **nftables only.** Rules are built as [`nftables`] schema objects and +//! exchanged with the kernel as JSON — never as text this file formats and the +//! machine parses back. Hand-written `nft` syntax has to be re-parsed to be +//! read, and scraping `nft list` output means a node's safety depends on the +//! output format of whatever nftables version an operator happens to have. +//! `iptables` was supported in an earlier draft and dropped: it cannot express +//! the layer 2 rule at all (that is `ebtables`, a third tool), it has no +//! equivalent of a typed exchange, and a second code path enforcing "the same" +//! policy is a second code path to get subtly wrong. + +use std::net::IpAddr; + +use anyhow::{Context, Result, bail}; +use async_trait::async_trait; +use nftables::batch::Batch; +use nftables::expr::{Expression, Meta, MetaKey, NamedExpression, Payload, PayloadField}; +use nftables::schema::{Chain, NfListObject, Nftables, Rule, Set, SetTypeValue, Table}; +use nftables::stmt::{Counter, JumpTarget, Match, Operator, Statement}; +use nftables::types::{NfChainPolicy, NfChainType, NfFamily, NfHook}; +use serde::{Deserialize, Serialize}; + +use crate::net::{DesiredDataPlane, GUEST_BRIDGE, TUNNEL_INTERFACE}; + +/// The name of everything this module owns. +/// +/// One name, so an operator looking at their own firewall can see at a glance +/// which parts are LNVPS's, and so the daemon can delete its own work without +/// having to remember what it created. +pub const TABLE: &str = "lnvps"; + +/// The chain every packet from a guest is checked against first. +const SOURCE_CHAIN: &str = "source"; + +/// One guest, as the filter sees it. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +pub struct Binding { + pub address: IpAddr, + /// The guest's MAC, when LNVPS assigned one. + /// + /// A guest can set its own MAC, so this is not a strong identity — but + /// pairing it with the address means a spoofing guest has to get both right + /// *and* still cannot use an address that belongs to a guest on another + /// node. Binding to the switch port instead is stronger and arrives in + /// increment 5, when the daemon starts creating the ports. + pub mac: Option, +} + +/// What the filter should enforce, derived from LNVPS's data plane document. +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct Policy { + pub bindings: Vec, +} + +impl Policy { + /// Read a policy out of the data plane document. + /// + /// The guest list is LNVPS's, never the node's own view of what is on the + /// bridge: a node that derived the allowed addresses from the interfaces it + /// could see would authorise whatever an operator attached. + pub fn from_desired(desired: &DesiredDataPlane) -> Result { + let mut bindings = Vec::new(); + for guest in &desired.guests { + let address = guest + .address + .split('/') + .next() + .unwrap_or_default() + .parse::() + .with_context(|| { + format!("LNVPS sent {}, which is not an address", guest.address) + })?; + bindings.push(Binding { + address, + // Lower-cased because that is how nftables states a MAC, and a + // policy that differs from the machine's only by case would + // read as drift on every comparison. + mac: guest.mac.as_ref().map(|m| m.to_lowercase()), + }); + } + bindings.sort(); + bindings.dedup(); + Ok(Self { bindings }) + } + + fn of(&self, v6: bool) -> impl Iterator { + self.bindings + .iter() + .filter(move |b| b.address.is_ipv6() == v6) + } +} + +/// A short, stable name for a policy. +/// +/// Rendered into a rule comment and read back off the machine, which is what +/// makes a refresh a no-op when nothing has changed. Derived from the policy +/// rather than from the built ruleset, so a change in *how* the rules are +/// expressed does not masquerade as a change in who is allowed. +pub fn fingerprint(policy: &Policy) -> String { + let joined = policy + .bindings + .iter() + .map(|b| format!("{}@{}", b.address, b.mac.as_deref().unwrap_or("-"))) + .collect::>() + .join(","); + format!( + "lnvps:{}", + &crate::control_auth::sha256_hex(joined.as_bytes())[..16] + ) +} + +/// What the filter currently is on this machine. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct FirewallState { + /// Whether this machine can filter at all. `false` is a fact the health + /// gate needs rather than an error to retry: a node with no nftables is not + /// a node with a broken one, and the difference is what the operator reads. + pub available: bool, + /// Whether LNVPS's ruleset is currently loaded. + pub present: bool, + /// Whether guest-to-guest traffic is blocked at layer 2. + /// + /// Separate from `present` because it is a separate table with a separate + /// hook, and a node with IP filtering but no L2 isolation is a real state + /// worth naming rather than rounding down to "filtered". + pub isolated: bool, + /// How many guest addresses the loaded ruleset admits, for LNVPS's own + /// reporting: a node whose count does not match the document it was sent is + /// a node whose filter is stale, and a stale filter is how a returned + /// address stays usable. + pub bindings: usize, + /// Which ruleset is loaded, as the machine states it. + /// + /// Carried in a rule comment rather than remembered by the daemon, because + /// the question is what the *kernel* is enforcing. A daemon that trusted + /// its own memory would keep reporting the ruleset it once applied after an + /// operator flushed the table by hand. + pub ruleset: Option, + /// Packets dropped for claiming an address the guest was not assigned. + /// + /// Reported because it is the one number here that says something about a + /// *customer* rather than about the node: a guest that is spoofing is + /// either compromised or hostile, and LNVPS would rather find out from a + /// counter than from an upstream abuse report. + pub spoofed_packets: u64, +} + +/// Talking to nftables. +/// +/// A trait for the same reason [`crate::net::NetOps`] is one: the policy and +/// the decisions are worth testing without root, and the end-to-end harness +/// needs to drive the real thing inside a namespace. Both directions are typed +/// — a ruleset in, a ruleset out — so nothing here formats or parses nft +/// syntax. +#[async_trait] +pub trait FirewallOps: Send + Sync { + /// Whether this machine has a working nftables. + async fn available(&self) -> bool; + /// Load a ruleset, atomically. + async fn apply(&self, ruleset: &Nftables<'_>) -> Result<()>; + /// Read the machine's current ruleset back. + async fn ruleset(&self) -> Result>; +} + +/// Load `policy` onto the machine, replacing whatever was there before. +/// +/// A machine already running this exact ruleset is left alone. Reloading would +/// be harmless — the swap is atomic — but it would mean the daemon reported a +/// change on every poll, and a log in which everything changes constantly is a +/// log in which nothing can be noticed. +pub async fn apply(ops: &dyn FirewallOps, policy: &Policy) -> Result> { + if !ops.available().await { + bail!( + "This machine has no working nftables, so guests cannot be filtered \ + and none will be placed here" + ); + } + let tag = fingerprint(policy); + if observe(ops).await.ruleset.as_deref() == Some(tag.as_str()) { + return Ok(Vec::new()); + } + ops.apply(&ruleset(policy)) + .await + .context("Cannot load the guest firewall")?; + Ok(vec![format!( + "loaded firewall {tag} with {} guest bindings", + policy.bindings.len() + )]) +} + +/// Read the loaded ruleset back. +/// +/// Every field is taken from the machine's own JSON rather than from anything +/// the daemon remembers, so an operator who flushes the table by hand is +/// reported as unfiltered — and gets it rebuilt on the next refresh. +pub async fn observe(ops: &dyn FirewallOps) -> FirewallState { + if !ops.available().await { + return FirewallState::default(); + } + let Ok(current) = ops.ruleset().await else { + return FirewallState { + available: true, + ..Default::default() + }; + }; + let mut state = FirewallState { + available: true, + ..Default::default() + }; + for object in current.objects.iter() { + let NfObjectRef::List(object) = object.into() else { + continue; + }; + match object { + NfListObject::Chain(chain) + if chain.table == TABLE + && chain.family == NfFamily::Bridge + && chain.policy == Some(NfChainPolicy::Drop) => + { + state.isolated = true; + } + NfListObject::Chain(chain) if chain.table == TABLE && chain.hook.is_some() => { + state.present = true; + } + // The set the return path is checked against: its size is how many + // guests this machine will currently deliver to. + NfListObject::Set(set) if set.table == TABLE && set.name.starts_with("assigned") => { + state.bindings += set.elem.as_ref().map(|e| e.len()).unwrap_or(0); + } + NfListObject::Rule(rule) if rule.table == TABLE => { + let Some(comment) = rule.comment.as_deref().filter(|c| c.starts_with("lnvps:")) + else { + continue; + }; + state.ruleset = Some(comment.to_string()); + // The counter on the same rule as the tag, so a counter added + // elsewhere — by a later version of this file, or by an + // operator — is never reported as spoofing. + for statement in rule.expr.iter() { + if let Statement::Counter(Counter::Anonymous(Some(counter))) = statement { + state.spoofed_packets = counter.packets.unwrap_or(0) as u64; + } + } + } + _ => {} + } + } + state +} + +/// Borrowed view of an object in a ruleset, list or command. +/// +/// nftables states input as commands and output as bare objects; observation +/// only ever sees the latter, but matching on both means a future caller +/// reading back its own transaction does not silently see nothing. +enum NfObjectRef<'a, 'b> { + List(&'a NfListObject<'b>), + Other, +} + +impl<'a, 'b> From<&'a nftables::schema::NfObject<'b>> for NfObjectRef<'a, 'b> { + fn from(object: &'a nftables::schema::NfObject<'b>) -> Self { + match object { + nftables::schema::NfObject::ListObject(o) => NfObjectRef::List(o), + nftables::schema::NfObject::CmdObject(_) => NfObjectRef::Other, + } + } +} + +/// The whole ruleset, as one transaction. +/// +/// nftables applies a batch atomically, so the delete and the rebuild either +/// both happen or neither does. The alternative — flushing and then adding — +/// leaves a window in which guests are unfiltered, which on a machine carrying +/// other people's customers is not a window worth having. +pub fn ruleset(policy: &Policy) -> Nftables<'static> { + let mut batch = Batch::new(); + let tag = fingerprint(policy); + + // Added before it is deleted, so the delete has something to delete: + // nftables fails a transaction that removes what does not exist, which + // would mean the very first run on a freshly booted node always errored. + for family in [NfFamily::INet, NfFamily::Bridge] { + batch.add(NfListObject::Table(Table { + family, + name: TABLE.into(), + handle: None, + })); + batch.delete(NfListObject::Table(Table { + family, + name: TABLE.into(), + handle: None, + })); + batch.add(NfListObject::Table(Table { + family, + name: TABLE.into(), + handle: None, + })); + } + + // Guests LNVPS gave a MAC are held to the pair; guests without one are + // allowed on address alone. Two sets rather than one, so it is visible in + // the loaded ruleset which guests are held to which standard. + for v6 in [false, true] { + let family = if v6 { "6" } else { "4" }; + batch.add(NfListObject::Set(Box::new(set( + &format!("bound{family}"), + SetTypeValue::Concatenated( + vec![nftables::schema::SetType::EtherAddr, address_type(v6)].into(), + ), + policy + .of(v6) + .filter_map(|b| { + b.mac.as_ref().map(|mac| { + Expression::Named(NamedExpression::Concat(vec![ + Expression::String(mac.clone().into()), + Expression::String(b.address.to_string().into()), + ])) + }) + }) + .collect(), + )))); + batch.add(NfListObject::Set(Box::new(set( + &format!("guest{family}"), + SetTypeValue::Single(address_type(v6)), + policy + .of(v6) + .filter(|b| b.mac.is_none()) + .map(|b| Expression::String(b.address.to_string().into())) + .collect(), + )))); + batch.add(NfListObject::Set(Box::new(set( + &format!("assigned{family}"), + SetTypeValue::Single(address_type(v6)), + policy + .of(v6) + .map(|b| Expression::String(b.address.to_string().into())) + .collect(), + )))); + } + + batch.add(NfListObject::Chain(Chain { + family: NfFamily::INet, + table: TABLE.into(), + name: SOURCE_CHAIN.into(), + newname: None, + handle: None, + dev: None, + _type: None, + hook: None, + prio: None, + policy: None, + })); + for v6 in [false, true] { + batch.add(rule( + SOURCE_CHAIN, + vec![ + Statement::Match(Match { + left: Expression::Named(NamedExpression::Concat(vec![ + field("ether", "saddr"), + field(protocol(v6), "saddr"), + ])), + right: Expression::String( + format!("@bound{}", if v6 { "6" } else { "4" }).into(), + ), + op: Operator::EQ, + }), + Statement::Return(None), + ], + None, + )); + batch.add(rule( + SOURCE_CHAIN, + vec![ + Statement::Match(Match { + left: field(protocol(v6), "saddr"), + right: Expression::String( + format!("@guest{}", if v6 { "6" } else { "4" }).into(), + ), + op: Operator::EQ, + }), + Statement::Return(None), + ], + None, + )); + } + // The drop that carries the tag and the counter. Both live here because + // this is the rule that says "a guest lied about who it is": the tag makes + // the ruleset identifiable, and the counter makes the lying visible. + batch.add(rule( + SOURCE_CHAIN, + vec![ + Statement::Counter(Counter::Anonymous(None)), + Statement::Drop(None), + ], + Some(tag), + )); + + batch.add(NfListObject::Chain(hooked( + NfFamily::INet, + "forward", + NfHook::Forward, + ))); + + // Checked before anything else, including established connections: an + // address that has been returned to the pool may already be another + // customer's, and a flow opened while it was still ours must not outlive + // the assignment. + batch.add(rule( + "forward", + vec![ + iif(GUEST_BRIDGE), + Statement::Jump(JumpTarget { + target: SOURCE_CHAIN.into(), + }), + ], + None, + )); + + // Clamped to the route's MTU rather than to a number decided here. A guest + // that ignores path MTU discovery otherwise opens a connection that works + // until the first large transfer and then hangs, which is a far worse + // failure than a slightly small segment. + batch.add(rule("forward", vec![clamp_mss()], None)); + + batch.add(rule( + "forward", + vec![ + Statement::Match(Match { + left: Expression::Named(NamedExpression::CT(nftables::expr::CT { + key: "state".into(), + family: None, + dir: None, + })), + right: Expression::List(vec![ + Expression::String("established".into()), + Expression::String("related".into()), + ]), + op: Operator::IN, + }), + Statement::Accept(None), + ], + None, + )); + + // Out of the guest network and up the tunnel. Anything a guest sends is + // LNVPS-addressed by the rule above, so there is nowhere else for it to go. + batch.add(rule( + "forward", + vec![ + iif(GUEST_BRIDGE), + oif(TUNNEL_INTERFACE), + Statement::Accept(None), + ], + None, + )); + + for v6 in [false, true] { + let assigned = format!("@assigned{}", if v6 { "6" } else { "4" }); + // Back down the tunnel, but only to an address LNVPS actually placed + // here. The route server should not be sending anything else, and if it + // does, the node not delivering it is the cheaper mistake. + batch.add(rule( + "forward", + vec![ + iif(TUNNEL_INTERFACE), + oif(GUEST_BRIDGE), + daddr_in(v6, &assigned), + Statement::Accept(None), + ], + None, + )); + // Two guests on this node, routed rather than bridged. They can reach + // each other from different nodes, so refusing it here would make the + // network behave differently depending on where LNVPS happened to + // place them. + batch.add(rule( + "forward", + vec![ + iif(GUEST_BRIDGE), + oif(GUEST_BRIDGE), + daddr_in(v6, &assigned), + Statement::Accept(None), + ], + None, + )); + } + + batch.add(NfListObject::Chain(hooked( + NfFamily::INet, + "input", + NfHook::Input, + ))); + batch.add(rule( + "input", + vec![ + Statement::Match(Match { + left: Expression::Named(NamedExpression::Meta(Meta { key: MetaKey::Iif })), + right: Expression::String("lo".into()), + op: Operator::EQ, + }), + Statement::Accept(None), + ], + None, + )); + batch.add(rule( + "input", + vec![ + Statement::Match(Match { + left: Expression::Named(NamedExpression::CT(nftables::expr::CT { + key: "state".into(), + family: None, + dir: None, + })), + right: Expression::List(vec![ + Expression::String("established".into()), + Expression::String("related".into()), + ]), + op: Operator::IN, + }), + Statement::Accept(None), + ], + None, + )); + batch.add(rule( + "input", + vec![ + iif(GUEST_BRIDGE), + Statement::Jump(JumpTarget { + target: SOURCE_CHAIN.into(), + }), + ], + None, + )); + // The guest's own gateway: it must be able to resolve and ping it, or it + // has no working network and no way to say so. Neighbour discovery is not + // optional the way ICMPv4 arguably is — without it IPv6 does not function + // at all — and the same two rules serve the route server, which has to + // prove the node is reachable before customers are placed on it. + for interface in [GUEST_BRIDGE, TUNNEL_INTERFACE] { + batch.add(rule( + "input", + vec![iif(interface), icmp_types(false), Statement::Accept(None)], + None, + )); + batch.add(rule( + "input", + vec![iif(interface), icmp_types(true), Statement::Accept(None)], + None, + )); + } + + // A second table, in the bridge family, because layer 2 is a different path + // through the kernel: a frame from one guest to another never reaches the + // forward hook above. + batch.add(NfListObject::Chain(hooked( + NfFamily::Bridge, + "forward", + NfHook::Forward, + ))); + + batch.to_nftables() +} + +/// A named set, empty when there is nothing in it. +/// +/// Every node looks like that on its first day, and the rules referring to the +/// set still load and match nothing — which is the correct behaviour for a node +/// with no guests, and better than a ruleset that only exists once a customer +/// arrives. +fn set( + name: &str, + set_type: SetTypeValue<'static>, + elements: Vec>, +) -> Set<'static> { + Set { + family: NfFamily::INet, + table: TABLE.into(), + name: name.to_string().into(), + handle: None, + set_type, + policy: None, + flags: None, + // `None` rather than an empty list: nftables rejects an empty element + // expression, and a node with no guests — every node, on its first day + // — must still get a working ruleset. + elem: (!elements.is_empty()).then(|| elements.into()), + timeout: None, + gc_interval: None, + size: None, + comment: None, + } +} + +/// A chain the kernel calls, dropping anything no rule accepted. +fn hooked(family: NfFamily, name: &str, hook: NfHook) -> Chain<'static> { + Chain { + family, + table: TABLE.into(), + name: name.to_string().into(), + newname: None, + handle: None, + dev: None, + _type: Some(NfChainType::Filter), + hook: Some(hook), + prio: Some(0), + policy: Some(NfChainPolicy::Drop), + } +} + +fn rule( + chain: &str, + expr: Vec>, + comment: Option, +) -> NfListObject<'static> { + NfListObject::Rule(Rule { + family: NfFamily::INet, + table: TABLE.into(), + chain: chain.to_string().into(), + expr: expr.into(), + handle: None, + index: None, + comment: comment.map(Into::into), + }) +} + +fn field(protocol: &str, name: &str) -> Expression<'static> { + Expression::Named(NamedExpression::Payload(Payload::PayloadField( + PayloadField { + protocol: protocol.to_string().into(), + field: name.to_string().into(), + }, + ))) +} + +fn protocol(v6: bool) -> &'static str { + if v6 { "ip6" } else { "ip" } +} + +fn address_type(v6: bool) -> nftables::schema::SetType { + if v6 { + nftables::schema::SetType::Ipv6Addr + } else { + nftables::schema::SetType::Ipv4Addr + } +} + +fn iif(name: &str) -> Statement<'static> { + Statement::Match(Match { + left: Expression::Named(NamedExpression::Meta(Meta { + key: MetaKey::Iifname, + })), + right: Expression::String(name.to_string().into()), + op: Operator::EQ, + }) +} + +fn oif(name: &str) -> Statement<'static> { + Statement::Match(Match { + left: Expression::Named(NamedExpression::Meta(Meta { + key: MetaKey::Oifname, + })), + right: Expression::String(name.to_string().into()), + op: Operator::EQ, + }) +} + +fn daddr_in(v6: bool, set: &str) -> Statement<'static> { + Statement::Match(Match { + left: field(protocol(v6), "daddr"), + right: Expression::String(set.to_string().into()), + op: Operator::EQ, + }) +} + +/// `tcp option maxseg size set rt mtu`, for SYNs only. +fn clamp_mss() -> Statement<'static> { + Statement::Mangle(nftables::stmt::Mangle { + key: Expression::Named(NamedExpression::TcpOption(nftables::expr::TcpOption { + name: "maxseg".into(), + field: Some("size".into()), + })), + value: Expression::Named(NamedExpression::RT(nftables::expr::RT { + key: nftables::expr::RTKey::MTU, + family: None, + })), + }) +} + +/// The ICMP types a guest and the route server are allowed to send us. +fn icmp_types(v6: bool) -> Statement<'static> { + let types = if v6 { + vec![ + "echo-request", + "nd-neighbor-solicit", + "nd-neighbor-advert", + "nd-router-solicit", + ] + } else { + vec!["echo-request"] + }; + // An anonymous set, not a list: nftables reads a list as a bitmask, and an + // ICMP type is an enumeration rather than flags — it rejects the ruleset + // outright, which is the good outcome compared with matching nothing. + Statement::Match(Match { + left: field(if v6 { "icmpv6" } else { "icmp" }, "type"), + right: Expression::Named(NamedExpression::Set( + types + .into_iter() + .map(|t| nftables::expr::SetItem::Element(Expression::String(t.to_string().into()))) + .collect(), + )), + op: Operator::EQ, + }) +} + +/// A machine with no packet filter at all. +/// +/// The counterpart of [`crate::net::UnavailableKernel`], and not a mock: it is +/// what a node looks like before anything has been installed on it, and +/// reporting that truthfully is what makes the health gate say "this node +/// cannot filter its guests" rather than assuming it can. +pub struct UnavailableFirewall; + +#[async_trait] +impl FirewallOps for UnavailableFirewall { + async fn available(&self) -> bool { + false + } + + async fn apply(&self, _ruleset: &Nftables<'_>) -> Result<()> { + bail!("This machine has no nftables") + } + + async fn ruleset(&self) -> Result> { + bail!("This machine has no nftables") + } +} + +pub use system::SystemFirewall; + +mod system { + use std::io::Write as _; + use std::process::{Command, Stdio}; + + use super::*; + use crate::netns; + + /// The machine's own nftables, inside the data plane namespace. + /// + /// Inside, because a ruleset loaded in the machine's namespace would filter + /// the operator's traffic and not a single guest packet — the guests are + /// not there. nftables has no "in this namespace" argument, and the + /// [`nftables`] crate's own helpers spawn on whichever thread the runtime + /// picks, so the process is started from a thread that has already entered + /// the namespace and inherits it. + pub struct SystemFirewall { + namespace: netns::Handle, + } + + impl SystemFirewall { + pub fn new(namespace: netns::Handle) -> Self { + Self { namespace } + } + + /// Run `nft -j` with the given arguments, in the namespace. + fn nft(&self, args: &[&str], stdin: String) -> Result { + let args: Vec = args.iter().map(|a| a.to_string()).collect(); + self.namespace.enter(move || { + let mut child = Command::new("nft") + .arg("-j") + .args(&args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .context("Cannot run nft")?; + child + .stdin + .take() + .expect("stdin was piped") + .write_all(stdin.as_bytes()) + .context("Cannot write a ruleset to nft")?; + let out = child.wait_with_output().context("Cannot run nft")?; + if !out.status.success() { + // nftables' own complaint, not ours: it names the offending + // expression, which an exit code does not. + bail!( + "nft failed: {}", + String::from_utf8_lossy(&out.stderr).trim() + ); + } + Ok(String::from_utf8_lossy(&out.stdout).to_string()) + }) + } + } + + #[async_trait] + impl FirewallOps for SystemFirewall { + async fn available(&self) -> bool { + // Run it rather than looking for it on `PATH`: the question is + // whether it works, and an `nft` that cannot reach its kernel + // module is an `nft` this node does not have. + self.nft(&["list", "ruleset"], String::new()).is_ok() + } + + async fn apply(&self, ruleset: &Nftables<'_>) -> Result<()> { + let payload = serde_json::to_string(ruleset).context("Cannot encode the ruleset")?; + self.nft(&["-f", "-"], payload)?; + Ok(()) + } + + async fn ruleset(&self) -> Result> { + let listed = self.nft(&["list", "ruleset"], String::new())?; + serde_json::from_str(&listed).context("Cannot read the machine's ruleset") + } + } +} + +#[cfg(test)] +pub mod tests; diff --git a/lnvps_node/src/fw/tests.rs b/lnvps_node/src/fw/tests.rs new file mode 100644 index 00000000..d3b84478 --- /dev/null +++ b/lnvps_node/src/fw/tests.rs @@ -0,0 +1,527 @@ +//! What the filter decides, tested without root. +//! +//! Whether the rules actually drop a spoofing guest is proved by +//! `lnvps_e2e/tests/tunnel_netns.rs`, which loads them into a real kernel and +//! sends real packets. What is worth asserting here is the reasoning: which +//! addresses are admitted, in what order the checks happen, and what the node +//! says about machines that cannot filter. +//! +//! The assertions read the same typed objects the kernel is sent, so a test +//! that passes is a test about the ruleset rather than about a string. + +use std::sync::Mutex; + +use nftables::schema::{NfCmd, NfObject}; + +use super::*; +use crate::net::{DesiredDataPlane, DesiredGuest, DesiredTunnel}; + +/// A machine with nftables, holding whatever was last loaded onto it. +#[derive(Default)] +pub struct FakeFirewall { + /// Whether this machine has a working nftables at all. + pub has_nft: bool, + loaded: Mutex>>, + /// How many times a ruleset has been loaded, which is how "nothing changed" + /// is told apart from "changed to the same thing". + pub loads: Mutex, +} + +impl FakeFirewall { + pub fn with_nft() -> Self { + Self { + has_nft: true, + ..Default::default() + } + } + + /// The objects the kernel would be holding: what a transaction added, less + /// what it deleted. Enough for observation to be tested against the same + /// shape the real thing returns. + fn kernel_view(commands: &Nftables<'_>) -> Vec> { + let mut out = Vec::new(); + for object in commands.objects.iter() { + match object { + NfObject::CmdObject(NfCmd::Add(list)) => { + out.push(NfObject::ListObject(clone_static(list))) + } + NfObject::CmdObject(NfCmd::Delete(_)) => out.clear(), + _ => {} + } + } + out + } + + fn objects(&self) -> Vec> { + self.loaded.lock().unwrap().clone() + } +} + +/// The schema borrows, and a fake machine has to own what it was given. +fn clone_static(object: &NfListObject<'_>) -> NfListObject<'static> { + let json = serde_json::to_string(object).expect("the schema round-trips"); + serde_json::from_str(&json).expect("the schema round-trips") +} + +#[async_trait] +impl FirewallOps for FakeFirewall { + async fn available(&self) -> bool { + self.has_nft + } + + async fn apply(&self, ruleset: &Nftables<'_>) -> Result<()> { + if !self.has_nft { + bail!("no nftables on this machine"); + } + *self.loaded.lock().unwrap() = Self::kernel_view(ruleset); + *self.loads.lock().unwrap() += 1; + Ok(()) + } + + async fn ruleset(&self) -> Result> { + Ok(Nftables { + objects: self.objects().into(), + }) + } +} + +fn desired(guests: Vec) -> DesiredDataPlane { + DesiredDataPlane { + tunnel: DesiredTunnel { + address4: Some("10.66.0.2/32".to_string()), + address6: None, + gateway4: None, + gateway6: None, + server_public_key: "0".repeat(64), + endpoint: "198.51.100.1:51820".to_string(), + keepalive: Some(25), + mtu: 1420, + }, + gateways: vec!["203.0.113.1".to_string()], + guests, + } +} + +fn guest(address: &str, mac: Option<&str>) -> DesiredGuest { + DesiredGuest { + address: address.to_string(), + gateway: "203.0.113.1".to_string(), + mac: mac.map(|m| m.to_string()), + } +} + +fn policy(guests: Vec) -> Policy { + Policy::from_desired(&desired(guests)).unwrap() +} + +/// The rules in one chain, in the order the kernel will evaluate them. +fn chain_rules<'a, 'b>( + objects: &'a [NfObject<'b>], + family: NfFamily, + chain: &str, +) -> Vec<&'a Rule<'b>> { + objects + .iter() + .filter_map(|o| match o { + NfObject::ListObject(NfListObject::Rule(r)) + if r.chain == chain && r.family == family => + { + Some(r) + } + NfObject::CmdObject(NfCmd::Add(NfListObject::Rule(r))) + if r.chain == chain && r.family == family => + { + Some(r) + } + _ => None, + }) + .collect() +} + +fn set_elements(objects: &[NfObject<'_>], name: &str) -> Vec { + objects + .iter() + .filter_map(|o| match o { + NfObject::CmdObject(NfCmd::Add(NfListObject::Set(s))) if s.name == name => Some(s), + NfObject::ListObject(NfListObject::Set(s)) if s.name == name => Some(s), + _ => None, + }) + .flat_map(|s| s.elem.iter().flat_map(|e| e.iter())) + .map(|e| serde_json::to_string(e).unwrap()) + .collect() +} + +/// The policy is LNVPS's list, not the node's view of its own bridge: a node +/// that authorised whatever it found attached would authorise whatever the +/// operator attached. +#[test] +fn the_policy_comes_from_the_document() { + let policy = policy(vec![ + guest("203.0.113.5", Some("AA:BB:CC:DD:EE:FF")), + guest("2001:db8::5", None), + ]); + + assert_eq!(policy.bindings.len(), 2); + // Lower-cased, because that is how nftables states a MAC, and a ruleset + // that differs from the machine's only by case reads as drift on every + // single comparison. + assert_eq!( + policy.bindings[0].mac.as_deref(), + Some("aa:bb:cc:dd:ee:ff"), + "{policy:?}" + ); +} + +/// An address with a prefix is still an address. LNVPS sends guest addresses +/// both ways depending on the range, and a filter that only understood one form +/// would silently drop that guest's traffic. +#[test] +fn a_guest_address_may_carry_a_prefix() { + let policy = policy(vec![guest("203.0.113.5/24", None)]); + assert_eq!(policy.bindings[0].address.to_string(), "203.0.113.5"); +} + +/// A malformed address is named rather than skipped: quietly dropping the guest +/// from the policy would leave it running with no filter entry at all, which +/// fails open. +#[test] +fn a_malformed_address_is_refused() { + let err = Policy::from_desired(&desired(vec![guest("not-an-address", None)])).unwrap_err(); + assert!(err.to_string().contains("not-an-address"), "{err}"); +} + +/// A guest LNVPS gave a MAC is held to the pair; one without is admitted on +/// address alone, and visibly in the weaker set rather than through a hole in +/// the strong one. +#[test] +fn a_mac_binds_the_address_to_it() { + let objects = ruleset(&policy(vec![ + guest("203.0.113.5", Some("aa:bb:cc:dd:ee:ff")), + guest("203.0.113.6", None), + ])) + .objects + .to_vec(); + + let bound = set_elements(&objects, "bound4"); + assert_eq!(bound.len(), 1, "{bound:?}"); + assert!(bound[0].contains("aa:bb:cc:dd:ee:ff") && bound[0].contains("203.0.113.5")); + + let unbound = set_elements(&objects, "guest4"); + assert_eq!(unbound, vec!["\"203.0.113.6\"".to_string()]); + + // Both are deliverable to; only one may claim the stronger check. + assert_eq!(set_elements(&objects, "assigned4").len(), 2); +} + +/// The transaction deletes before it rebuilds, and adds the table first so the +/// delete has something to delete — otherwise the very first run on a freshly +/// booted node fails, which is the run that matters most. +#[test] +fn the_ruleset_is_replaced_atomically() { + let objects = ruleset(&Policy::default()).objects.to_vec(); + let table_ops: Vec<&'static str> = objects + .iter() + .filter_map(|o| match o { + NfObject::CmdObject(NfCmd::Add(NfListObject::Table(_))) => Some("add"), + NfObject::CmdObject(NfCmd::Delete(NfListObject::Table(_))) => Some("delete"), + _ => None, + }) + .collect(); + assert_eq!( + table_ops, + vec!["add", "delete", "add", "add", "delete", "add"], + "one family's table, then the other's" + ); +} + +/// A node with no guests still gets a complete, working ruleset. Every node +/// looks like this on its first day, and a filter that only appeared once a +/// customer arrived would leave the first one unprotected. +#[test] +fn a_node_with_no_guests_still_loads() { + let objects = ruleset(&Policy::default()).objects.to_vec(); + assert!(!chain_rules(&objects, NfFamily::INet, "forward").is_empty()); + assert!(set_elements(&objects, "assigned4").is_empty()); +} + +/// Anti-spoof is checked before conntrack. An address that has gone back in the +/// pool may already be someone else's, and a flow opened while it was still +/// this guest's must not outlive the assignment. +#[test] +fn spoofing_is_checked_before_established_connections() { + let objects = ruleset(&Policy::default()).objects.to_vec(); + let forward = chain_rules(&objects, NfFamily::INet, "forward"); + + let jump = forward + .iter() + .position(|r| r.expr.iter().any(|s| matches!(s, Statement::Jump(_)))) + .expect("guest traffic is checked"); + let conntrack = forward + .iter() + .position(|r| { + serde_json::to_string(&r.expr) + .unwrap() + .contains("established") + }) + .expect("established connections are accepted"); + assert!(jump < conntrack, "{forward:#?}"); +} + +/// Layer 2 is a different path through the kernel: a frame from one guest to +/// another never reaches the forward hook, so proxy ARP would otherwise let a +/// tenant poison their neighbours. +#[test] +fn guests_are_isolated_from_each_other_at_layer_two() { + let objects = ruleset(&Policy::default()).objects.to_vec(); + let bridge = objects.iter().find_map(|o| match o { + NfObject::CmdObject(NfCmd::Add(NfListObject::Chain(c))) if c.family == NfFamily::Bridge => { + Some(c) + } + _ => None, + }); + let bridge = bridge.expect("a bridge chain"); + assert_eq!(bridge.hook, Some(NfHook::Forward)); + assert_eq!(bridge.policy, Some(NfChainPolicy::Drop)); +} + +/// Clamped to the route's MTU rather than to a number decided when the rules +/// were built: the tunnel's MTU can change under the filter, and a stale clamp +/// hangs large transfers exactly like no clamp at all. +#[test] +fn the_mss_is_clamped_to_the_path() { + let objects = ruleset(&Policy::default()).objects.to_vec(); + let mangle = chain_rules(&objects, NfFamily::INet, "forward") + .iter() + .any(|r| { + r.expr.iter().any(|s| { + matches!(s, Statement::Mangle(m) if serde_json::to_string(&m.value).unwrap().contains("mtu")) + }) + }); + assert!(mangle, "{objects:#?}"); +} + +/// Return traffic is delivered only to addresses LNVPS placed here, and a guest +/// can reach a neighbour on the same node because it could reach one on a +/// different node. A network that behaved differently depending on where LNVPS +/// happened to place a VM would be a network nobody could reason about. +#[test] +fn only_assigned_addresses_are_delivered() { + let objects = ruleset(&policy(vec![guest("203.0.113.5", None)])) + .objects + .to_vec(); + let accepts: Vec = chain_rules(&objects, NfFamily::INet, "forward") + .iter() + .filter(|r| r.expr.iter().any(|s| matches!(s, Statement::Accept(_)))) + .map(|r| serde_json::to_string(&r.expr).unwrap()) + .collect(); + + assert!( + accepts.iter().any(|r| r.contains(TUNNEL_INTERFACE) + && r.contains(GUEST_BRIDGE) + && r.contains("@assigned4")), + "{accepts:#?}" + ); + assert!( + accepts + .iter() + .any(|r| r.matches(GUEST_BRIDGE).count() == 2 && r.contains("@assigned4")), + "two guests on one node should reach each other through it: {accepts:#?}" + ); +} + +/// Neighbour discovery is accepted. ICMPv4 is arguably optional; ICMPv6 is not, +/// and a node that dropped it would report a configured tunnel with no working +/// IPv6 guest behind it. +#[test] +fn neighbour_discovery_survives() { + let objects = ruleset(&Policy::default()).objects.to_vec(); + let input = serde_json::to_string(&chain_rules(&objects, NfFamily::INet, "input")).unwrap(); + assert!(input.contains("nd-neighbor-solicit"), "{input}"); + assert!(input.contains("echo-request"), "{input}"); +} + +/// A machine already running this exact ruleset is left alone. Reloading is +/// harmless — the swap is atomic — but a daemon that reported a change on every +/// poll would produce a log in which nothing can be noticed. +#[tokio::test] +async fn a_machine_already_filtered_is_left_alone() { + let fake = FakeFirewall::with_nft(); + let policy = policy(vec![guest("203.0.113.5", None)]); + + assert!(!apply(&fake, &policy).await.unwrap().is_empty()); + assert!(apply(&fake, &policy).await.unwrap().is_empty()); + assert_eq!(*fake.loads.lock().unwrap(), 1); +} + +/// A guest arriving or leaving changes the tag, so the next refresh reloads. +/// The tag is derived from the policy rather than from the built ruleset, so a +/// change in how the rules are expressed cannot masquerade as a change in who +/// is allowed. +#[test] +fn the_tag_follows_the_guests() { + let none = fingerprint(&Policy::default()); + let one = fingerprint(&policy(vec![guest("203.0.113.5", None)])); + let bound = fingerprint(&policy(vec![guest( + "203.0.113.5", + Some("aa:bb:cc:dd:ee:ff"), + )])); + + assert_ne!(none, one); + assert_ne!(one, bound, "binding a MAC is a different policy"); + assert!(none.starts_with("lnvps:")); +} + +/// The tag is read back off the machine rather than remembered, so an operator +/// who flushes the table by hand gets it rebuilt on the next refresh instead of +/// the daemon insisting it is already there. +#[tokio::test] +async fn a_flushed_table_is_rebuilt() { + let fake = FakeFirewall::with_nft(); + let policy = policy(vec![guest("203.0.113.5", None)]); + apply(&fake, &policy).await.unwrap(); + + *fake.loaded.lock().unwrap() = Vec::new(); + assert!(!apply(&fake, &policy).await.unwrap().is_empty()); + assert_eq!(*fake.loads.lock().unwrap(), 2); +} + +/// A machine running someone else's idea of the ruleset — an older daemon, a +/// half-applied change — is reloaded rather than accepted. +#[tokio::test] +async fn a_stale_ruleset_is_replaced() { + let fake = FakeFirewall::with_nft(); + apply(&fake, &policy(vec![guest("203.0.113.5", None)])) + .await + .unwrap(); + + let changed = apply(&fake, &policy(vec![guest("203.0.113.6", None)])) + .await + .unwrap(); + assert!(!changed.is_empty(), "a different guest list must reload"); + assert_eq!(set_elements(&fake.objects(), "assigned4").len(), 1); +} + +/// A machine with no nftables is refused outright rather than configured +/// without a filter: an unfiltered node is a node where one customer can be +/// another, and it is better for it to carry nobody. +#[tokio::test] +async fn a_machine_with_no_filter_is_refused() { + let err = apply(&FakeFirewall::default(), &Policy::default()) + .await + .unwrap_err(); + assert!(err.to_string().contains("no working nftables"), "{err}"); + assert!(err.to_string().contains("none will be placed"), "{err}"); +} + +/// Observation reads the machine, not what was applied: the case worth catching +/// is the one where the two disagree. +#[tokio::test] +async fn observation_reads_the_machine() { + let fake = FakeFirewall::with_nft(); + let policy = policy(vec![ + guest("203.0.113.5", Some("aa:bb:cc:dd:ee:ff")), + guest("2001:db8::5", None), + ]); + apply(&fake, &policy).await.unwrap(); + + let state = observe(&fake).await; + assert!(state.available); + assert!(state.present); + assert!( + state.isolated, + "the layer 2 table is part of being filtered" + ); + assert_eq!(state.bindings, 2, "one guest per family"); + assert_eq!(state.ruleset, Some(fingerprint(&policy))); + assert_eq!(state.spoofed_packets, 0, "nothing has lied yet"); +} + +/// A machine with nftables and no ruleset loaded reports exactly that, rather +/// than an error. It is the state a node is in before its first apply and after +/// somebody flushes the table by hand, and the health gate needs to tell it +/// apart from "no firewall at all". +#[tokio::test] +async fn an_unfiltered_machine_says_so() { + let state = observe(&FakeFirewall::with_nft()).await; + assert!(state.available); + assert!(!state.present); + assert!(!state.isolated); + assert_eq!(state.ruleset, None); + + // ...and a machine with no nftables at all is a third, distinct answer. + let state = observe(&FakeFirewall::default()).await; + assert!(!state.available); + assert!(!state.present); +} + +/// The drop counter is read off the rule that carries the tag, so a counter +/// added elsewhere — by a later version of this file, or by an operator — is +/// never reported as a customer spoofing. +#[tokio::test] +async fn spoofed_packets_are_counted() { + let fake = FakeFirewall::with_nft(); + apply(&fake, &Policy::default()).await.unwrap(); + + // The kernel filling in a counter, which is what the real one returns. + let mut objects = fake.objects(); + for object in objects.iter_mut() { + if let NfObject::ListObject(NfListObject::Rule(rule)) = object { + if rule.comment.is_some() { + let mut expr = rule.expr.to_vec(); + expr[0] = Statement::Counter(Counter::Anonymous(Some( + nftables::stmt::AnonymousCounter { + packets: Some(42), + bytes: Some(3528), + }, + ))); + rule.expr = expr.into(); + } + } + } + *fake.loaded.lock().unwrap() = objects; + + assert_eq!(observe(&fake).await.spoofed_packets, 42); +} + +/// A machine with nothing installed reports that, rather than pretending. It is +/// the state every node is in before the operator has finished setting it up, +/// and the integration tests run against it because it needs no root. +#[tokio::test] +async fn an_unequipped_machine_is_honest() { + let none = UnavailableFirewall; + assert!(!none.available().await); + assert!(none.apply(&ruleset(&Policy::default())).await.is_err()); + assert!(none.ruleset().await.is_err()); + assert_eq!(observe(&none).await, FirewallState::default()); +} + +/// A machine that cannot be read is reported as unfiltered, not as filtered. +/// Failing the other way would mean an unreadable node passing the health gate. +#[tokio::test] +async fn an_unreadable_machine_is_not_assumed_filtered() { + struct Unreadable; + + #[async_trait] + impl FirewallOps for Unreadable { + async fn available(&self) -> bool { + true + } + async fn apply(&self, _ruleset: &Nftables<'_>) -> Result<()> { + Ok(()) + } + async fn ruleset(&self) -> Result> { + bail!("nft: unable to talk to the kernel") + } + } + + // Loading works; it is only reading back that fails, which is the awkward + // case — the daemon has every reason to believe it succeeded. + Unreadable + .apply(&ruleset(&Policy::default())) + .await + .unwrap(); + + let state = observe(&Unreadable).await; + assert!(state.available, "the tool is there"); + assert!(!state.present, "but nothing can be claimed about the rules"); +} diff --git a/lnvps_node/src/lib.rs b/lnvps_node/src/lib.rs index 94b0e074..f4f46069 100644 --- a/lnvps_node/src/lib.rs +++ b/lnvps_node/src/lib.rs @@ -15,6 +15,8 @@ //! - [`api`] — outbound calls to LNVPS, the only direction that works before //! there is a tunnel. //! - [`net`] — applying the data plane LNVPS asked for, over netlink. +//! - [`fw`] — the packet filter around the guests, which is what stops one +//! customer being another. //! - [`netns`] — the namespace that data plane lives in, so LNVPS configures //! its own network rather than the operator's. //! - [`wgkey`] — the node's WireGuard key, generated here and never sent. @@ -25,6 +27,7 @@ pub mod config; pub mod control; pub mod control_auth; pub mod credential; +pub mod fw; pub mod inventory; pub mod net; pub mod netns; diff --git a/lnvps_node/src/main.rs b/lnvps_node/src/main.rs index e18f0d7b..7747c2fd 100644 --- a/lnvps_node/src/main.rs +++ b/lnvps_node/src/main.rs @@ -119,7 +119,11 @@ async fn dataplane(config_path: &Path, action: DataplaneAction) -> Result<()> { // have?" is the question an operator asks when something is wrong, and it // must not fail because the token is missing or LNVPS is unreachable. if let DataplaneAction::Observe = action { - let state = lnvps_node::net::observe(&lnvps_node::net::Kernel::new()?).await?; + let state = lnvps_node::net::observe( + &lnvps_node::net::Kernel::new()?, + &lnvps_node::fw::SystemFirewall::new(lnvps_node::netns::ensure_default()?), + ) + .await?; println!("{}", serde_json::to_string_pretty(&state)?); return Ok(()); } @@ -141,7 +145,8 @@ async fn dataplane(config_path: &Path, action: DataplaneAction) -> Result<()> { DataplaneAction::Show => println!("{}", serde_json::to_string_pretty(&desired)?), DataplaneAction::Apply => { let kernel = lnvps_node::net::Kernel::new()?; - let applied = lnvps_node::net::apply(&kernel, &desired, &key).await?; + let fw = lnvps_node::fw::SystemFirewall::new(lnvps_node::netns::ensure_default()?); + let applied = lnvps_node::net::apply(&kernel, &fw, &desired, &key).await?; for line in applied { println!("{line}"); } @@ -173,7 +178,10 @@ async fn run(config_path: &Path) -> Result<()> { // control API binds the tunnel interface, and on a fresh machine the tunnel // does not exist until now. let kernel = Arc::new(lnvps_node::net::Kernel::new()?); - if let Err(e) = apply_dataplane(&config, kernel.as_ref()).await { + let fw = Arc::new(lnvps_node::fw::SystemFirewall::new( + lnvps_node::netns::ensure_default()?, + )); + if let Err(e) = apply_dataplane(&config, kernel.as_ref(), fw.as_ref()).await { // Not fatal. A node whose tunnel is already up from a previous run must // keep serving through an LNVPS outage — refusing to start would turn // an API blip into every node on the platform going dark. @@ -209,11 +217,14 @@ async fn run(config_path: &Path) -> Result<()> { // startup would route a departed customer's address until it was restarted. let refresh = config.clone(); let refresh_kernel = kernel.clone(); + let refresh_fw = fw.clone(); tokio::spawn(async move { let interval = Duration::from_secs(refresh.heartbeat_secs.max(10)); loop { tokio::time::sleep(interval).await; - if let Err(e) = apply_dataplane(&refresh, refresh_kernel.as_ref()).await { + if let Err(e) = + apply_dataplane(&refresh, refresh_kernel.as_ref(), refresh_fw.as_ref()).await + { log::warn!("Data plane refresh failed: {e}"); } } @@ -228,7 +239,7 @@ async fn run(config_path: &Path) -> Result<()> { })?; control::serve_on( - Arc::new(ControlState::new(control_pubkey, addr, kernel)), + Arc::new(ControlState::new(control_pubkey, addr, kernel, fw)), listener, tls, ) @@ -236,7 +247,11 @@ async fn run(config_path: &Path) -> Result<()> { } /// Fetch the data plane and apply it. -async fn apply_dataplane(config: &NodeConfig, kernel: &dyn lnvps_node::net::NetOps) -> Result<()> { +async fn apply_dataplane( + config: &NodeConfig, + kernel: &dyn lnvps_node::net::NetOps, + fw: &dyn lnvps_node::fw::FirewallOps, +) -> Result<()> { let credential = Credential::load_checked(&config.credential)?; let api = lnvps_node::api::LnvpsApi::new(&config.api_url, &credential)?; @@ -250,7 +265,7 @@ async fn apply_dataplane(config: &NodeConfig, kernel: &dyn lnvps_node::net::NetO api.request_tunnel(&key.public_bytes()).await?; let desired = api.dataplane().await?; - let applied = lnvps_node::net::apply(kernel, &desired, &key).await?; + let applied = lnvps_node::net::apply(kernel, fw, &desired, &key).await?; if !applied.is_empty() { log::debug!("Applied data plane: {}", applied.join("; ")); } diff --git a/lnvps_node/src/net.rs b/lnvps_node/src/net.rs index 7ebdcaad..87b565ab 100644 --- a/lnvps_node/src/net.rs +++ b/lnvps_node/src/net.rs @@ -165,6 +165,9 @@ pub struct DataPlaneState { pub forwarding6: bool, /// Guest addresses actually routed to the bridge. pub routed_guests: usize, + /// The packet filter around those guests. + #[serde(default)] + pub firewall: crate::fw::FirewallState, } impl DataPlaneState { @@ -173,8 +176,17 @@ impl DataPlaneState { /// A handshake is required, not just an interface: WireGuard comes up happily /// with a peer that never answers, and a node in that state looks /// configured while being unreachable. + /// + /// The filter is required for a different reason: a node that forwards + /// without one is a node where any guest can source any address and reach + /// any neighbour. That is worse than a node that carries nobody, so it is + /// counted as unhealthy rather than as a warning. pub fn healthy(&self) -> bool { - self.tunnel_up && self.last_handshake_secs.is_some() && self.bridge_up && self.forwarding4 + self.tunnel_up + && self.last_handshake_secs.is_some() + && self.bridge_up + && self.forwarding4 + && self.firewall.present } } @@ -186,12 +198,18 @@ impl DataPlaneState { /// the first. pub async fn apply( ops: &dyn NetOps, + fw: &dyn crate::fw::FirewallOps, desired: &DesiredDataPlane, key: &NodeKey, ) -> Result> { let mut changed = Vec::new(); apply_tunnel(ops, desired, key, &mut changed).await?; apply_bridge(ops, desired, &mut changed).await?; + + // The filter before forwarding, every time. Between the two the machine + // routes guest traffic with nothing checking it, and on a refresh that adds + // a guest that window is exactly when the new guest starts sending. + changed.extend(crate::fw::apply(fw, &crate::fw::Policy::from_desired(desired)?).await?); apply_forwarding(ops, &mut changed).await?; Ok(changed) } @@ -425,7 +443,7 @@ fn is_link_local(address: &IpNetwork) -> bool { } /// Read back what the machine actually has. -pub async fn observe(ops: &dyn NetOps) -> Result { +pub async fn observe(ops: &dyn NetOps, fw: &dyn crate::fw::FirewallOps) -> Result { let (tunnel_up, tunnel_mtu) = ops.link_state(TUNNEL_INTERFACE).await?; let (bridge_up, _) = ops.link_state(GUEST_BRIDGE).await?; let last_handshake_secs = ops @@ -440,6 +458,7 @@ pub async fn observe(ops: &dyn NetOps) -> Result { forwarding4: enabled(ops, "net/ipv4/ip_forward").await?, forwarding6: enabled(ops, "net/ipv6/conf/all/forwarding").await?, routed_guests: ops.routes(GUEST_BRIDGE).await?.len(), + firewall: crate::fw::observe(fw).await, }) } diff --git a/lnvps_node/src/net/tests.rs b/lnvps_node/src/net/tests.rs index dc68f194..49fda05a 100644 --- a/lnvps_node/src/net/tests.rs +++ b/lnvps_node/src/net/tests.rs @@ -10,6 +10,16 @@ use std::sync::Mutex; use super::*; +/// A machine with a working nftables, already carrying the ruleset for the +/// guests below, for the tests that are about the network rather than the +/// filter. The filter's own decisions are tested in [`crate::fw::tests`]. +async fn fw() -> crate::fw::tests::FakeFirewall { + let fake = crate::fw::tests::FakeFirewall::with_nft(); + let policy = crate::fw::Policy::from_desired(&desired()).unwrap(); + crate::fw::apply(&fake, &policy).await.unwrap(); + fake +} + /// A machine that remembers what it was told, and answers questions from what /// it has been told so far — so a second `apply` sees the first one's work, /// which is what makes "converges and then goes quiet" testable. @@ -239,7 +249,9 @@ fn cidr(value: &str) -> IpNetwork { #[tokio::test] async fn a_bare_machine_gets_the_whole_data_plane() { let kernel = FakeKernel::new(); - let changed = apply(&kernel, &desired(), &key()).await.unwrap(); + let changed = apply(&kernel, &fw().await, &desired(), &key()) + .await + .unwrap(); assert!(!changed.is_empty()); assert_eq!( @@ -303,9 +315,13 @@ async fn a_bare_machine_gets_the_whole_data_plane() { #[tokio::test] async fn a_correct_machine_is_not_touched_again() { let kernel = FakeKernel::new(); - apply(&kernel, &desired(), &key()).await.unwrap(); + apply(&kernel, &fw().await, &desired(), &key()) + .await + .unwrap(); - let changed = apply(&kernel, &desired(), &key()).await.unwrap(); + let changed = apply(&kernel, &fw().await, &desired(), &key()) + .await + .unwrap(); // Configuring WireGuard is stated unconditionally — the kernel API takes // the whole interface and there is nothing to compare a private key // against — but nothing else may move. @@ -330,7 +346,9 @@ async fn a_stale_peer_is_removed() { .peers .push(("c3RyYXk=".to_string(), None)); - let changed = apply(&kernel, &desired(), &key()).await.unwrap(); + let changed = apply(&kernel, &fw().await, &desired(), &key()) + .await + .unwrap(); assert!( changed .iter() @@ -351,13 +369,17 @@ async fn a_stale_peer_is_removed() { #[tokio::test] async fn a_departed_guest_stops_being_routed() { let kernel = FakeKernel::new(); - apply(&kernel, &desired(), &key()).await.unwrap(); + apply(&kernel, &fw().await, &desired(), &key()) + .await + .unwrap(); kernel .add_route(cidr("203.0.113.9/32"), GUEST_BRIDGE) .await .unwrap(); - let changed = apply(&kernel, &desired(), &key()).await.unwrap(); + let changed = apply(&kernel, &fw().await, &desired(), &key()) + .await + .unwrap(); assert!( changed .iter() @@ -378,7 +400,9 @@ async fn a_departed_guest_stops_being_routed() { #[tokio::test] async fn a_stale_address_goes_and_the_kernels_own_stays() { let kernel = FakeKernel::new(); - apply(&kernel, &desired(), &key()).await.unwrap(); + apply(&kernel, &fw().await, &desired(), &key()) + .await + .unwrap(); kernel .add_address(TUNNEL_INTERFACE, cidr("10.66.0.9/32")) .await @@ -388,7 +412,9 @@ async fn a_stale_address_goes_and_the_kernels_own_stays() { .await .unwrap(); - apply(&kernel, &desired(), &key()).await.unwrap(); + apply(&kernel, &fw().await, &desired(), &key()) + .await + .unwrap(); let addresses = kernel.addresses_of(TUNNEL_INTERFACE); assert!(!addresses.contains(&cidr("10.66.0.9/32")), "{addresses:?}"); assert!(addresses.contains(&cidr("fe80::1/64")), "{addresses:?}"); @@ -402,7 +428,7 @@ async fn a_single_stack_tunnel_only_routes_its_own_family() { let kernel = FakeKernel::new(); let mut plane = desired(); plane.tunnel.address6 = None; - apply(&kernel, &plane, &key()).await.unwrap(); + apply(&kernel, &fw().await, &plane, &key()).await.unwrap(); assert_eq!(kernel.routes_of(TUNNEL_INTERFACE), vec![cidr("0.0.0.0/0")]); } @@ -417,7 +443,9 @@ async fn a_kernel_without_ipv6_still_configures() { ], ..FakeKernel::new() }; - apply(&kernel, &desired(), &key()).await.unwrap(); + apply(&kernel, &fw().await, &desired(), &key()) + .await + .unwrap(); assert_eq!( kernel.sysctl_value("net/ipv4/ip_forward"), Some("1".to_string()) @@ -431,12 +459,16 @@ async fn a_malformed_address_is_reported() { let kernel = FakeKernel::new(); let mut plane = desired(); plane.tunnel.address4 = Some("not-an-address".to_string()); - let err = apply(&kernel, &plane, &key()).await.unwrap_err(); + let err = apply(&kernel, &fw().await, &plane, &key()) + .await + .unwrap_err(); assert!(format!("{err:#}").contains("not-an-address"), "{err:#}"); let mut plane = desired(); plane.gateways = vec!["also-not".to_string()]; - let err = apply(&kernel, &plane, &key()).await.unwrap_err(); + let err = apply(&kernel, &fw().await, &plane, &key()) + .await + .unwrap_err(); assert!(format!("{err:#}").contains("also-not"), "{err:#}"); } @@ -445,11 +477,13 @@ async fn a_malformed_address_is_reported() { #[tokio::test] async fn observation_reports_what_the_machine_has() { let kernel = FakeKernel::new(); - apply(&kernel, &desired(), &key()).await.unwrap(); + apply(&kernel, &fw().await, &desired(), &key()) + .await + .unwrap(); // WireGuard comes up happily with a peer that never answers, so an interface // that has never handshaken is configured, not working. - let state = observe(&kernel).await.unwrap(); + let state = observe(&kernel, &fw().await).await.unwrap(); assert!(state.tunnel_up); assert_eq!(state.tunnel_mtu, Some(1420)); assert_eq!(state.last_handshake_secs, None); @@ -469,7 +503,7 @@ async fn observation_reports_what_the_machine_has() { .get_mut(TUNNEL_INTERFACE) .unwrap() .peers = vec![("peer".to_string(), Some(12))]; - let state = observe(&kernel).await.unwrap(); + let state = observe(&kernel, &fw().await).await.unwrap(); assert_eq!(state.last_handshake_secs, Some(12)); assert!(state.healthy()); } @@ -479,7 +513,12 @@ async fn observation_reports_what_the_machine_has() { #[tokio::test] async fn an_unconfigured_machine_observes_cleanly() { let kernel = FakeKernel::new(); - let state = observe(&kernel).await.unwrap(); + // No packet filter either: a machine nobody has configured has not had one + // installed, and reporting one would be reporting a protection it does not + // have. + let state = observe(&kernel, &crate::fw::UnavailableFirewall) + .await + .unwrap(); assert_eq!(state, DataPlaneState::default()); assert!(!state.healthy()); } diff --git a/lnvps_node/tests/control_https.rs b/lnvps_node/tests/control_https.rs index 289e5422..1c5f2672 100644 --- a/lnvps_node/tests/control_https.rs +++ b/lnvps_node/tests/control_https.rs @@ -38,6 +38,7 @@ async fn start_node(keys: &Keys, state_dir: &std::path::Path) -> (SocketAddr, Ve keys.public_key(), addr, Arc::new(lnvps_node::net::UnavailableKernel), + Arc::new(lnvps_node::fw::UnavailableFirewall), )); tokio::spawn(async move { serve(state, addr, node_tls).await }); diff --git a/work/marketplace.md b/work/marketplace.md index 98464e6f..0c2f8b5d 100644 --- a/work/marketplace.md +++ b/work/marketplace.md @@ -840,9 +840,14 @@ deliberately: `wg-quick` files for something else to read. A marketplace node runs on hardware LNVPS does not own, so a data plane that depends on the operator having wired it up correctly is one whose mistakes surface as a customer's VM having no network. The daemon re-converges instead. -- **Both `nft` and `iptables` are supported**, detected at runtime, because a node is somebody - else's machine and refusing the ones that run iptables would refuse real hardware. It costs a - second dialect, which is why the firewall is its own increment. +- **nftables only, spoken as JSON through the `nftables` crate** — not `iptables`, and not `nft` + syntax this codebase formats and then parses back. Rules go to the kernel as typed objects and + come back the same way, so what the daemon reports is what the kernel holds rather than what a + scraper made of `nft list` output on whichever version the operator has. `iptables` was in the + first draft and dropped: it cannot express the layer 2 rule at all (that is `ebtables`, a third + tool), it has no typed exchange, and a second code path enforcing "the same" policy is a second + code path to get subtly wrong. Debian has shipped `nftables` by default since Buster (2019); + a machine without it is refused, which is the correct answer for a machine that cannot filter. - **The health gate spawns a real guest and pings it** rather than asking the node how it thinks it is doing. The node self-reporting "bridge up, forwarding on" cannot catch a bridge with no path to the tunnel, which is exactly the mistake worth catching before a customer @@ -937,13 +942,54 @@ unreachable" for every node in the pool. - `lnvps-node dataplane show|apply` so an operator can see and re-drive it without the daemon. - `/api/v1/status` reports the observed data plane, which 4c3's gate reads as its first check. -#### 4c2 — Anti-spoof + anti-LAN firewall (M/L) ⬅ NEXT -- One ruleset the daemon owns wholesale, in `nft` where available and `iptables` where not, - with the backend detected once and reported in status. -- Guests may not reach the operator's own LAN or the node's management addresses; may not - source traffic as an address LNVPS did not assign them; MSS clamped to the tunnel's MTU. -- The guest address set comes from 4c1's document, so the boundary is LNVPS's list, not - something the node infers. +#### 4c2 — Anti-spoof + guest isolation (M/L) ✅ +Re-scoped after 4c1: the namespace already did the anti-LAN half. There is no path from +`br-lnvps` to the operator's network to block, so the rules that remain are the ones no +topology can enforce. + +- **Anti-spoof, bound to the MAC where there is one.** A guest may source only the addresses + LNVPS assigned it. The route server's `AllowedIPs` stops node A pretending to be node B; this + stops guest A pretending to be guest B *on the same node*, which `AllowedIPs` cannot see + because both addresses legitimately belong to that node's peer. +- **Guests may not reach each other at L2.** They share one bridge, and with proxy ARP they + believe every address is on-link, so without this a tenant can ARP-poison, ND-poison or + DHCP-spoof their neighbours — attacks that never reach the IP layer where the rest of the + ruleset lives. Dropping the bridge's forward hook leaves guest-to-guest traffic to be + *routed* by the node, which is exactly where it can be filtered. It is also what they would + get if the two guests were on different nodes, so it is the consistent answer, not a + restriction. +- **MSS clamped to the path MTU** on forwarded SYNs, so a guest that ignores path MTU discovery + gets a connection that works rather than one that opens and hangs. +- **The ruleset is owned wholesale and replaced atomically.** The daemon does not add rules to + the operator's chains: it renders a complete table and swaps it in one transaction, so there + is no window in which a guest is unfiltered and no way for a half-applied set to persist. +- **nftables only, and typed.** The ruleset is built as `nftables` crate schema objects and + exchanged with the kernel as JSON in both directions — never as text this codebase formats and + the machine parses back. A node whose nftables does not work is refused rather than configured + without a filter. +- The guest list comes from 4c1's document, so the boundary is LNVPS's, not something the node + infers from what it happens to see on the bridge. + +Deferred to increment 5, where the daemon starts creating taps: per-port `isolated` flags and +per-tap filtering. Binding an address to a *port* is stronger than binding it to a MAC, which a +guest chooses. Until the daemon owns the ports it cannot do this, and MAC binding plus L2 +isolation is what is available in the meantime. + +Built as `lnvps_node/src/fw.rs`, and two decisions are worth recording: + +- **The machine states which ruleset it is running, and the daemon believes it.** The tag is + carried in a rule comment and read back out of the kernel's own JSON, so an + operator who flushes the table by hand gets it rebuilt on the next refresh. A daemon that + remembered what it last applied would go on reporting a filter that no longer existed — + which is the failure this whole increment exists to prevent, arrived at from the other side. +- **Nothing is reloaded when nothing has changed.** Reloading would be harmless, since every + backend swaps atomically, but a daemon that reported a change on every poll produces a log + in which nothing can be noticed. + +The end-to-end harness proves the drop rather than the ping: a spoofed packet fails to get a +reply anyway for want of a return route, so a failed ping proves nothing. What proves it is the +drop counter moving — the packet stopped on the node, before the tunnel, LNVPS's network, or an +upstream that would attribute it to the operator. #### 4c3 — Health gate (M/L) - A probe guest is provisioned through the ordinary path onto the new node, given a real