diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 14d1df9..dd347b7 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -4,8 +4,19 @@ updates: directory: "/" schedule: interval: weekly + # Transitive crates too: a root daemon parsing untrusted packets is only + # as current as its whole dependency tree. + allow: + - dependency-type: all + groups: + # Minor and patch bumps arrive as one PR; majors stay separate. + cargo-minor: + update-types: ["minor", "patch"] - package-ecosystem: github-actions directory: "/" schedule: interval: weekly + groups: + actions: + patterns: ["*"] diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml new file mode 100644 index 0000000..28b4e82 --- /dev/null +++ b/.github/workflows/e2e.yml @@ -0,0 +1,37 @@ +name: e2e + +on: + push: + branches: [main] + pull_request: + branches: [main] + +permissions: + contents: read + +env: + CARGO_TERM_COLOR: always + +jobs: + armed-e2e: + name: armed e2e (real NFQUEUE verdicts in a netns) + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: dtolnay/rust-toolchain@02cb101ec7c40f2c49e1d9714d64511d8e1b74de # master + with: + toolchain: stable + - name: Install build and test deps + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends \ + protobuf-compiler libnfnetlink-dev libnetfilter-queue-dev nftables jq + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + with: + key: armed-e2e + - name: Armed e2e (build, netns, shipped nft snippet, daemon, curl) + timeout-minutes: 25 + run: ./scripts/armed-e2e.sh diff --git a/.github/workflows/ebpf.yml b/.github/workflows/ebpf.yml index fed9e30..8e76eb4 100644 --- a/.github/workflows/ebpf.yml +++ b/.github/workflows/ebpf.yml @@ -513,19 +513,6 @@ jobs: grep -o 'verified_insns: [^ ]* = [0-9]*' guest.log | while read -r _ p _ n; do echo "| \`${p}\` verified insns | ${n} |" done || true - # The fast path's facts as this guest observed them and the - # decision the eligibility ladder takes on them with the feature - # on. Not what `cfc status` would say on a real host of this - # kernel: the guest mounts no bpffs, so `lifecycle_pinned` is - # false here on every kernel and the deadline shown is the reduced - # one wherever a real host would pin. The other two facts - - # `exit_precise` and the capability - are the kernel's own. - # `tr -d '\r'`: the guest console writes CRLF, and a CR inside a - # table cell ends the markdown row early. `|| true` as above: a - # guest that died before printing this already failed on the marker. - grep -o 'fast path on this kernel: .*' guest.log | tr -d '\r' | tail -1 | while read -r line; do - echo "| fast path | ${line#fast path on this kernel: } |" - done || true } >> "${GITHUB_STEP_SUMMARY}" # Wall clock, not just instruction count. They are different diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2265047..69cd189 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -21,6 +21,9 @@ jobs: - name: Resolve version and verify it matches the tag id: version + env: + REF_TYPE: ${{ github.ref_type }} + REF_NAME: ${{ github.ref_name }} run: | VERSION="$(sed -n '/^\[workspace\.package\]/,/^\[/{s/^version *= *"\(.*\)"/\1/p}' Cargo.toml | head -n1)" if [ -z "${VERSION}" ]; then @@ -30,8 +33,8 @@ jobs: # The release is named after the tag, but every asset filename is # built from the Cargo version. A mismatch publishes "v0.3.0" # containing colony-firewall-control-0.2.0-*.tar.zst. Refuse. - if [ "${{ github.ref_type }}" = "tag" ] && [ "${{ github.ref_name }}" != "v${VERSION}" ]; then - echo "::error::tag ${{ github.ref_name }} does not match Cargo.toml [workspace.package] version ${VERSION} (expected tag v${VERSION}). Bump Cargo.toml or retag." + if [ "${REF_TYPE}" = "tag" ] && [ "${REF_NAME}" != "v${VERSION}" ]; then + echo "::error::tag ${REF_NAME} does not match Cargo.toml [workspace.package] version ${VERSION} (expected tag v${VERSION}). Bump Cargo.toml or retag." exit 1 fi echo "version=${VERSION}" >> "${GITHUB_OUTPUT}" @@ -189,7 +192,7 @@ jobs: crates/cfc-ebpf/target/bpfel-unknown-none/release/cfc-ebpf.o \ "${STAGE}/" - # Docs, for parity with what the AUR package puts in + # Docs, for parity with what pkg/PKGBUILD puts in # /usr/share/doc. `cfc status` points users at TROUBLESHOOTING.md # by name, so it has to actually ship. install -m644 \ @@ -236,8 +239,9 @@ jobs: SHA256SUMS RELEASE_BODY.md - # Build pkg/PKGBUILD for real against the freshly pushed tag, and produce - # the AUR-submittable artifacts (PKGBUILD with real checksums + .SRCINFO). + # Build pkg/PKGBUILD for real against the freshly pushed tag, and attach + # the Arch packaging recipe (PKGBUILD with real checksums + .SRCINFO) to + # the release. The project is not published on the AUR. # # Hard gate, no continue-on-error: at tag time the source= URL # (.../archive/v$pkgver.tar.gz) resolves, because GitHub generates the @@ -275,10 +279,12 @@ jobs: chown -R builder: . - name: Verify pkgver matches the tag + env: + REF_NAME: ${{ github.ref_name }} run: | PKGVER="$(sed -n 's/^pkgver=//p' pkg/PKGBUILD | head -n1)" - if [ "${{ github.ref_name }}" != "v${PKGVER}" ]; then - echo "::error::pkg/PKGBUILD pkgver=${PKGVER} does not match tag ${{ github.ref_name }}" + if [ "${REF_NAME}" != "v${PKGVER}" ]; then + echo "::error::pkg/PKGBUILD pkgver=${PKGVER} does not match tag ${REF_NAME}" exit 1 fi @@ -287,7 +293,7 @@ jobs: run: | runuser -u builder -- updpkgsums PKGBUILD # A remote (non-VCS) source with sha256sums=('SKIP') is not - # acceptable AUR practice: it disables integrity checking of the + # acceptable Arch packaging practice: it disables integrity checking of the # release tarball entirely. if grep -qE "^sha256sums=\(.*'SKIP'" PKGBUILD; then echo "::error::pkg/PKGBUILD still has a SKIP checksum after updpkgsums" @@ -308,8 +314,8 @@ jobs: # An undotted copy is what gets attached: GitHub renames dot-leading # asset filenames (`.SRCINFO` became `default.SRCINFO`), so the # published name never matched what the README told people to - # download. Ship a name GitHub keeps; the AUR checkout renames it - # back to `.SRCINFO` locally. + # download. Ship a name GitHub keeps; whoever builds from it renames + # it back to `.SRCINFO` locally. cp .SRCINFO SRCINFO - name: namcap the PKGBUILD @@ -359,7 +365,7 @@ jobs: exit 1 fi - - name: Upload AUR artifacts + - name: Upload Arch packaging artifacts uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: aur-assets @@ -394,7 +400,7 @@ jobs: files: | release-assets/colony-firewall-control-*.tar.zst release-assets/SHA256SUMS - - name: Attach AUR artifacts + - name: Attach Arch packaging artifacts if: github.ref_type == 'tag' uses: softprops/action-gh-release@efb35369e0ad2afab669f228072c1b0d510eae64 # v3.0.3 with: diff --git a/CHANGELOG.md b/CHANGELOG.md index 23beae5..1e166db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,16 @@ and [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Removed + +- The Fast Allow userspace path, disabled since 0.7.0 because a socket mark + cannot prove which process sends and so opened bypasses. `cfc --json status` + no longer has a `fast_allow` key, `StatusResponse` field 16 is reserved, and + the `[ebpf] fast_allow` and `fast_allow_mark` keys are ignored with a + warning. For hosts upgrading from 0.4-0.6, startup still flushes the legacy + nftables set, disarms the legacy pinned maps and removes the old sendmsg + link pins. + ## [0.7.0] - 2026-09-30 ### Added diff --git a/README.md b/README.md index 7aec5b6..01806f1 100644 --- a/README.md +++ b/README.md @@ -68,17 +68,22 @@ NFQUEUE in the kernel, per-app pop-ups in iced, gRPC IPC over a Unix socket. +--------------------------------------------------+ ``` -Seven workspace crates: - -| Crate | Role | -|---------------|----------------------------------------------------------| -| `cfc-core` | Shared types: `Rule`, `Verdict`, `Connection`, `Process` | -| `cfc-proto` | gRPC schema (tonic + tonic-prost) | -| `cfc-client` | Shared UDS gRPC client wrapper | -| `cfc-daemon` | Privileged daemon | -| `cfc-ui` | iced GUI | -| `cfc-cli` | Terminal control tool | -| `cfc-tray` | System-tray companion (StatusNotifierItem) | +Ten crates: nine workspace members, plus the kernel-side `cfc-ebpf`, which +is its own workspace (pinned nightly + bpf-linker, built by `cargo xtask +build-ebpf`) so stable builds never see it: + +| Crate | Role | +|-------------------|----------------------------------------------------------------------------| +| `cfc-core` | Shared types and rule matching: `Rule`, `Verdict`, `Connection`, `Process` | +| `cfc-proto` | gRPC schema (tonic + tonic-prost) | +| `cfc-client` | Shared UDS gRPC client wrapper | +| `cfc-daemon` | Privileged daemon | +| `cfc-ui` | iced GUI | +| `cfc-cli` | Terminal control tool | +| `cfc-tray` | System-tray companion (StatusNotifierItem) | +| `cfc-ebpf-common` | POD types and pure parsers shared by eBPF and userspace | +| `cfc-ebpf` | Kernel-side programs of the optional eBPF backend | +| `xtask` | Build automation (eBPF object build) | More docs: @@ -171,7 +176,7 @@ Enable the installed daemon and enforcement in First run below. ## First run A fresh install has **zero rules**: once enforcement is on, every new -outbound connection prompts (or falls back to the profile default). Do +remote outbound connection prompts (or falls back to the profile default). Do these three things, in order: **1. Enable enforcement persistently.** A companion unit loads the @@ -204,11 +209,10 @@ systemd-timesyncd and chronyd NTP (:123/udp), the DHCP clients (dhcpcd, NetworkManager and systemd-networkd, :67 and :547/udp), pacman and paru HTTPS mirrors (:443/tcp), and the SSH client (:22/tcp) - and is idempotent (already-present rules are skipped by name; `--dry-run` -previews). **Do not skip this step.** No profile allows anything on its -own, so on a machine with no rules and no UI connected nothing outbound -gets through - including the DHCP lease. Filtering starts before the -network is configured (see below), and these rules are what let the -machine come up at all. +previews). **Do not skip this step.** No profile allows unmatched remote flows +on its own. With no rules and no UI connected, unmatched queued remote +connections are denied. Filtering starts before the network is configured +(see below), and these rules keep DHCP, DNS and NTP usable. For everything else, there are bundles: @@ -240,8 +244,8 @@ On a headless machine, answer them from the terminal instead: cfc prompts ``` -With no subscriber at all the daemon applies `no_ui_action` to every -unmatched flow without asking anyone. **That is a denial under every +With no subscriber at all the daemon applies `no_ui_action` to unmatched +remote flows without asking anyone. **That is a denial under every profile.** "Nobody is connected" is a permanent condition on a headless box, not a passing one, and answering it with an allow would mean those hosts had no outbound firewall whatsoever. Stored rules are what such a @@ -267,13 +271,25 @@ initramfs, interfaces already configured before these units, other network managers, or a later external ruleset flush. Early unmatched flows use `no_ui_action`; bootstrap DHCP/DNS/NTP rules keep strict configurations usable. -**Scope.** Rules decide new tracked flows; established and related traffic -retains its connection-wide authorization. Passed or inherited sockets and -local DNS/proxy relays are not confined to their original executable. -Loopback is exempt, and packet-layer traffic from applications with -`CAP_NET_RAW` is outside these IP hooks. Use OS containment for those cases. -Fast Allow is disabled even when `fast_allow = true` is configured; allowed -flows use the normal NFQUEUE path. +**Scope.** Normal mode decides new tracked IP flows from socket attribution; +established and related traffic retains its connection-wide authorization. +Passed or inherited sockets are not reauthorized for each sending executable. +A current descriptor holder does not prove which process sent a packet. +While the daemon runs, new direct loopback flows follow explicit rules; +unmatched local IPC is allowed without prompting. While no daemon listens on +the queue, new loopback flows are allowed (`queue ... bypass` on `lo` only), so +local services keep working; resolving names that are not cached still needs +the daemon. An allowed local resolver or proxy can still relay remote +traffic. CFC cannot establish the originating application's identity from +remote flows delegated through local brokers, including AF_UNIX and D-Bus. + +Applications with `CAP_NET_RAW` can use AF_PACKET outside the `inet OUTPUT` +hook. Raw IP packets can also coincide with another socket's tuple; socket +attribution does not prove their origin. Use explicit application confinement +or OS containment for those cases. +Fast Allow was removed: a socket mark cannot prove which process sends, so it +opened bypasses. The old `[ebpf] fast_allow` and `fast_allow_mark` keys are +ignored with a warning, and allowed flows use the normal NFQUEUE path. Then confirm it is really filtering: @@ -282,8 +298,9 @@ cfc status # "enforcing yes", and it warns on stderr when it is not ``` > **WARNING - remote / SSH machines:** the shipped nftables snippet is -> fail-closed. If the daemon is down while the rule is loaded, **all new -> outbound connections drop**, and a mistake can lock you out of a box you +> fail-closed for everything except new loopback flows, which are allowed +> while no daemon listens. If the daemon is down while the rule is loaded, +> **all new non-loopback outbound connections drop**, and a mistake can lock you out of a box you > only reach over SSH. Read > [docs/TROUBLESHOOTING.md](docs/TROUBLESHOOTING.md) - specifically the > SSH exemption and dead-man's-switch patterns - *before* enabling @@ -291,6 +308,9 @@ cfc status # "enforcing yes", and it warns on stderr when it is not ### Explicit application confinement +**Experimental.** This mode is new in 0.7.0, has not been externally +audited, and its interface and platform requirements may change. + `cfc applications run` starts a separate, headless application tree with an empty network permission list. Administrators may approve exact numeric peer addresses with `--allow IP`. Permissions apply to the entire tree across diff --git a/SECURITY.md b/SECURITY.md index 1de02eb..0c9dc9a 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,10 +1,14 @@ # Security Policy -Colony Firewall Control is **alpha software**. It runs a daemon as root with +Colony Firewall Control is **beta software**. It runs a daemon as root with `CAP_NET_ADMIN` and makes allow/deny decisions about your network traffic, so security reports are taken seriously -- but expectations should match the -project's maturity: there has been no external audit, and interfaces may -change without notice. +project's maturity: **there has been no external security audit yet**, and +interfaces may change without notice. + +The explicit application confinement mode (`cfc applications run`, new in +0.7.0) is **experimental**, and its interface and platform requirements may +change. ## Supported Versions diff --git a/TODO.md b/TODO.md index 09e472c..9b87283 100644 --- a/TODO.md +++ b/TODO.md @@ -20,15 +20,17 @@ longer lifts anything; `nft delete table` no longer lifts the denies it holds. Two pieces of it are deliberately not done, and both are real work rather than oversights: -**1a. Fast Allow is disabled.** Socket marks do not attest the current sender, -and grants can outlive their intended executable or rule. Every configuration, -including `fast_allow = true`, uses NFQUEUE for allowed flows. The nft snippet -no longer accepts the legacy set, startup clears old state, and upgrades reload -active nft units atomically. Reintroducing an in-kernel Allow requires a design -that verifies current socket ownership and revocation; the old mark protocol is -not a supported security boundary. - -The previous latency measurements describe the disabled implementation. The +**1a. Fast Allow was removed.** It let a process a lasting Allow covered skip +NFQUEUE by marking its sockets. A socket mark does not attest the current +sender, and grants could outlive their intended executable or rule, so it +opened bypasses. It was disabled in 0.7.0 and its userspace side has since been +removed; allowed flows use NFQUEUE. The nft snippet no longer accepts the +legacy set, startup flushes it and disarms the legacy pinned maps the kernel +object still carries until an ABI bump, and upgrades reload active nft units +atomically. Reintroducing an in-kernel Allow needs sender attestation: a design +that verifies current socket ownership and revocation. + +The previous latency measurements describe the removed implementation. The remaining NFQUEUE cost still warrants measurement and optimization, with the same application-policy semantics. @@ -45,14 +47,12 @@ be resolved to addresses in advance. Mostly done in `8db949b` and `b05eefc`: the SELinux module, the RPM provenance backend, the `.spec`, and a 5.10 entry in the kernel matrix that sits *below* -RHEL 9's backported 5.14. The fast-allow branch adds 5.15 above it, so the pair +RHEL 9's backported 5.14. The matrix also carries 5.15 above it, so the pair brackets the RHEL kernel: what both allow, 5.14 allows unless Red Hat took it out; what only 5.15 allows, 5.14 has only if they backported it; what both refuse, 5.14 may still have through a backport. Where the two disagree is the -list of things to check on a Rocky host rather than assume. The first 5.15 run -named one such thing: 5.15 already accepts `bpf_getsockopt` on the sendmsg hooks -that 5.10 refuses, so whether RHEL 9's 5.14 does is exactly what a Rocky host -has to answer; neither kernel has `group_dead`. +list of things to check on a Rocky host rather than assume. Neither kernel has +`group_dead`. What remains needs a real enforcing machine - except 2b, which turned out to be doable from CI after all: @@ -243,15 +243,16 @@ What defeats it completely: |---|---| | **Root** | narrower than it was, and still open. `nft delete table` no longer lifts the denials held in the kernel - those need `rm -rf /sys/fs/bpf/colony-firewall` as well, and anything not yet decided still falls through to a ruleset root can flush. CFC *is* root; it cannot confine root. | | **Code inside an allowed process** | a browser extension, a script under an allowed interpreter, `ptrace`/`LD_PRELOAD` injection. Structural to every application firewall. Making Allow persistent (`72964b5`) improved usability and widened this. | -| **Loopback** | `oifname "lo" accept`, deliberately - filtering it stalls the systemd-resolved stub. Anything that can reach a local service which egresses is attributed to that service. | +| **Loopback** | `oifname "lo" ct state new queue num 0 bypass`: the daemon judges new loopback flows while it runs (unmatched local IPC is allowed without prompting), and they are allowed unfiltered while no daemon listens, so local IPC survives a dead daemon (the systemd-resolved stub answers from its cache; its upstream queries are not loopback). In that window explicit loopback Deny rules are not enforced and nothing is logged. Anything that can reach a local service which egresses is attributed to that service. | | **DNS tunnelling** | the resolver must be allowed for anything to work. CFC *observes* answers; it does not inspect or block queries. | | **Inherited or passed socket descriptors** | Existing connection authorization is not rechecked for each sending executable; socket attribution is ambiguous when ownership is shared. | | **CAP_NET_RAW packet sockets** | Packet-layer egress can bypass the IP OUTPUT hook. Layer-2 confinement is outside the shipped rules. | | **Prompt fatigue** | demonstrated on this machine: ten Firefox prompts in a row, all denied, browser lost. A malicious installer generating thirty prompts trains the user to click Allow. | -And one tradeoff worth stating plainly: the ruleset is **fail-closed** (`ct -state new queue num 0`, no `bypass`). Killing the daemon drops all new outbound -traffic. That is the right choice for confidentiality and the wrong one for +And one tradeoff worth stating plainly: the ruleset is **fail-closed for +everything except new loopback flows, which are allowed while no daemon +listens** (the final `ct state new queue num 0` has no `bypass`). Killing the +daemon drops all new non-loopback outbound traffic. That is the right choice for confidentiality and the wrong one for availability - anything that can crash the daemon takes the machine's network with it. diff --git a/crates/cfc-cli/src/main.rs b/crates/cfc-cli/src/main.rs index d29a9d9..7eaf85d 100644 --- a/crates/cfc-cli/src/main.rs +++ b/crates/cfc-cli/src/main.rs @@ -397,7 +397,6 @@ struct StatusJson { uptime_seconds: u64, enforcing: bool, enforcement: String, - fast_allow: String, paused: bool, resume_at_unix_ms: i64, resume_at: Option, @@ -420,7 +419,6 @@ fn status_json(s: &proto::StatusResponse, now_unix_ms: i64) -> StatusJson { uptime_seconds: s.uptime_seconds, enforcing: s.enforcing, enforcement: s.enforcement.clone(), - fast_allow: s.fast_allow.clone(), paused: s.paused, resume_at_unix_ms: s.resume_at_unix_ms, resume_at: output::rfc3339(s.resume_at_unix_ms), @@ -481,19 +479,6 @@ fn enforcement_cell(level: &str) -> String { } } -/// The fast-allow cell: the daemon's own sentence, or why there is none. -/// -/// The daemon already spells this one out (`live`, or `off: ` and the reason -/// the path is inert), so the CLI only has to name the value the daemon cannot -/// send: the proto3 default, which is what a daemon without the field answers -/// and also what a daemon whose startup has not decided yet answers. -fn fast_allow_cell(level: &str) -> String { - match level { - "" => "unknown (still starting, or this daemon is too old to say)".to_string(), - other => other.to_string(), - } -} - fn paused_cell(s: &proto::StatusResponse, now_unix_ms: i64) -> String { if !s.paused { return "no".to_string(); @@ -527,7 +512,6 @@ async fn cmd_status(client: &mut Client, format: OutputFormat) -> CliResult { if s.enforcing { "yes" } else { "no" } ); println!(" in-kernel {}", enforcement_cell(&s.enforcement)); - println!(" fast-allow {}", fast_allow_cell(&s.fast_allow)); println!("paused {}", paused_cell(&s, now)); println!("rules {}", s.rules_count); println!("prompts pending {}", s.prompts_pending); @@ -614,7 +598,6 @@ mod tests { skipped_rules: 0, enforcing: true, enforcement: "pinned".to_string(), - fast_allow: "live".to_string(), } } @@ -772,34 +755,12 @@ mod tests { assert_eq!(v["warnings"].as_array().unwrap().len(), 2); } - /// The daemon's sentence passes through untouched in both output modes; - /// only its absence gets words, and those must not read as an answer. + /// The proto3 default is what an older daemon sends, and what a new one + /// sends before startup has answered; the text mode must say what the + /// blank means rather than print nothing. #[test] - fn fast_allow_passes_through_and_names_its_own_absence() { - let now = 1_700_000_000_000; - let mut s = status(false, 0); - let v = serde_json::to_value(status_json(&s, now)).unwrap(); - assert_eq!(v["fast_allow"], "live"); - - s.fast_allow = "off: [ebpf] fast_allow is not set".to_string(); - let v = serde_json::to_value(status_json(&s, now)).unwrap(); - assert_eq!(v["fast_allow"], "off: [ebpf] fast_allow is not set"); - assert_eq!(fast_allow_cell(&s.fast_allow), s.fast_allow); - assert_eq!(fast_allow_cell("live"), "live"); - - // The proto3 default is what an older daemon sends, and what a new - // one sends before startup has answered. JSON keeps it verbatim so a - // script can tell "" from a real value; the text mode says what the - // blank means, as the enforcement cell does for its own. - s.fast_allow = String::new(); - let v = serde_json::to_value(status_json(&s, now)).unwrap(); - assert_eq!(v["fast_allow"], ""); - let cell = fast_allow_cell(""); - assert!(cell.starts_with("unknown ("), "{cell}"); - assert!( - enforcement_cell("").starts_with("unknown ("), - "the two cells must agree on how an absent answer reads" - ); + fn an_absent_enforcement_level_reads_as_unknown() { + assert!(enforcement_cell("").starts_with("unknown (")); } #[test] diff --git a/crates/cfc-cli/tests/cli_e2e.rs b/crates/cfc-cli/tests/cli_e2e.rs index 16cc220..60b027f 100644 --- a/crates/cfc-cli/tests/cli_e2e.rs +++ b/crates/cfc-cli/tests/cli_e2e.rs @@ -146,7 +146,6 @@ impl Firewall for FakeDaemon { skipped_rules: 2, enforcing: false, enforcement: "pinned".to_string(), - fast_allow: "off: [ebpf] fast_allow is not set".to_string(), })) } @@ -570,9 +569,8 @@ async fn status_json_round_trips_over_a_real_socket() { assert_eq!(v["enforcing"], false); assert_eq!(v["skipped_rules"], 2); assert_eq!(v["timeout_action"], "deny"); - // The daemon's own sentence, verbatim: a script must be able to read - // the reason, not only that there is one. - assert_eq!(v["fast_allow"], "off: [ebpf] fast_allow is not set"); + // The removed Fast Allow field must not come back under its old name. + assert!(v.get("fast_allow").is_none(), "{v}"); // Both warnings must be machine-readable too, not just printed. let warnings = v["warnings"].as_array().expect("warnings array"); assert_eq!(warnings.len(), 2, "{warnings:?}"); diff --git a/crates/cfc-daemon/src/config.rs b/crates/cfc-daemon/src/config.rs index 1cdda51..a5df462 100644 --- a/crates/cfc-daemon/src/config.rs +++ b/crates/cfc-daemon/src/config.rs @@ -156,8 +156,10 @@ impl Profile { /// /// The outbound table cannot lock an operator out of a remote machine: it /// hooks `output` on `ct state new` only, so an inbound SSH session's - /// replies are `ct state established` and are never queued, and loopback is - /// accepted outright. Rules can still be added with `cfc-cli` from that + /// replies are `ct state established` and are never queued. While the + /// daemon runs, new loopback flows follow explicit policy and unmatched + /// local IPC is allowed without prompting; while no daemon listens, the + /// snippet's `bypass` on `lo` allows them. Rules can still be added with `cfc-cli` from that /// session. What it *does* mean on a fresh headless install is that /// outbound traffic — package updates, NTP, backups — is denied until /// rules exist for it. @@ -317,22 +319,13 @@ pub struct EbpfConfig { /// Where the BPF object built by `cargo xtask build-ebpf` was installed. /// `None` means `crate::ebpf::DEFAULT_OBJECT_PATH`. pub object_path: Option, - /// Compatibility setting, currently ignored: Fast Allow is disabled for - /// every configuration because socket marks cannot attest the sender. - /// Ordinary traffic uses NFQUEUE; the startup report explains the refusal. + /// Legacy key from the removed Fast Allow path. Ignored; a warning is + /// logged. Still parsed rather than rejected: a parse error stops the + /// daemon while the fail-closed nftables table stays loaded, so an + /// upgrade would take the host offline. pub fast_allow: bool, - /// The `SO_MARK` value the fast path uses, when the machine needs a - /// specific one. - /// - /// `None` - the default - draws one at random at each start, which is what - /// keeps it from being a forgeable token. Set it only to resolve a - /// collision: the mark space is shared with the whole machine, and a - /// consumer that selects on a *mask* will match a random value with a - /// probability its mask decides. See `ebpf::loader::pick_mark` for the - /// selectors CFC already avoids, and `docs/TROUBLESHOOTING.md` for how to - /// find the one it does not know about. - /// - /// Zero is refused: it is the mark of every socket nothing has marked. + /// Legacy key from the removed Fast Allow path. Ignored; a warning is + /// logged. pub fast_allow_mark: Option, } @@ -387,9 +380,10 @@ impl<'de> Deserialize<'de> for EbpfMode { /// `Auto`, matching how `profile` already treats an unknown value, and for /// a reason specific to this daemon: a config parse error propagates out of /// `Config::load` and the process exits *before* `READY=1`. The nftables - /// ruleset is `ct state new queue num 0` with no `bypass`, so a loaded - /// table with no daemon behind it blackholes every new outbound connection - /// on the machine. A typo in an enrichment layer's switch must not cost + /// ruleset is fail-closed for everything except new loopback flows, which + /// are allowed while no daemon listens: the final `ct state new queue num + /// 0` has no `bypass`, so a loaded table with no daemon behind it + /// blackholes every new non-loopback outbound connection on the machine. A typo in an enrichment layer's switch must not cost /// someone their network. fn deserialize>(d: D) -> Result { struct V; @@ -652,9 +646,10 @@ enabled = " Auto ""# /// A typo must not be able to take the machine's network away. /// /// A config parse error propagates out of `Config::load` and the daemon - /// exits *before* `READY=1`. `systemd/nftables-snippet.conf` is - /// `ct state new queue num 0` with **no** `bypass`, so a loaded table with - /// no daemon behind it drops every new outbound connection. Refusing to + /// exits *before* `READY=1`. `systemd/nftables-snippet.conf` ends with + /// `ct state new queue num 0` with **no** `bypass` (only the loopback rule + /// above it has one), so a loaded table with no daemon behind it drops + /// every new non-loopback outbound connection. Refusing to /// start over a misspelled enrichment-layer switch would turn a one-letter /// mistake into an outage, so an unknown value warns and falls back - /// exactly as `profile` already does. @@ -670,42 +665,15 @@ enabled = " Auto ""# assert_eq!(cfg.ebpf.enabled, EbpfMode::Auto); } - /// `daemon.toml.sample` documents the mark in hex, so hex has to parse. - /// A sample that shows a spelling the parser rejects is worse than no - /// sample: the operator only finds out when the daemon refuses to start. + /// The removed Fast Allow keys must keep parsing: rejecting them would + /// stop the daemon on upgrade with the fail-closed table still loaded. #[test] - fn the_fast_allow_mark_parses_in_the_spelling_the_sample_documents() { - let mark = |toml: &str| Config::from_toml_str(toml).unwrap().ebpf.fast_allow_mark; - assert_eq!(mark(""), None, "absent means draw one"); - assert_eq!( - mark("[ebpf]\nfast_allow_mark = 0x00033331\n"), - Some(0x0003_3331) - ); - assert_eq!(mark("[ebpf]\nfast_allow_mark = 209713\n"), Some(209_713)); - // The whole word must fit: the mark is a u32, and the top bit is as - // legitimate a mark bit as any other. - assert_eq!( - mark("[ebpf]\nfast_allow_mark = 0xffffffff\n"), - Some(u32::MAX) - ); - } - - /// The fast path is opt-in: nothing short of `fast_allow = true` turns it - /// on, and the layer's own eligibility checks still get the last word. - #[test] - fn ebpf_fast_allow_is_off_unless_asked_for() { - let fast_allow = |toml: &str| Config::from_toml_str(toml).unwrap().ebpf.fast_allow; - - assert!(!fast_allow(""), "absent means off"); - assert!( - !fast_allow("[ebpf]\n"), - "an empty section keeps the default" - ); - assert!(!fast_allow("[ebpf]\nfast_allow = false\n")); - assert!(fast_allow("[ebpf]\nfast_allow = true\n")); - // Parsed independently of `enabled`: the switch says what was asked - // for, and the layer decides whether it can honour it. - assert!(fast_allow("[ebpf]\nenabled = false\nfast_allow = true\n")); + fn the_legacy_fast_allow_keys_still_parse() { + let cfg = + Config::from_toml_str("[ebpf]\nfast_allow = true\nfast_allow_mark = 0x00033331\n") + .expect("legacy keys must not abort startup"); + assert!(cfg.ebpf.fast_allow); + assert_eq!(cfg.ebpf.fast_allow_mark, Some(0x0003_3331)); } #[test] @@ -902,8 +870,8 @@ enabled = " Auto ""# config resolves to the automatic default" ); assert!( - !cfg.ebpf.fast_allow, - "a shipped config must not take allows off the packet path" + !cfg.ebpf.fast_allow && cfg.ebpf.fast_allow_mark.is_none(), + "a shipped config must not set the legacy Fast Allow keys" ); assert_eq!( cfg.storage.path, diff --git a/crates/cfc-daemon/src/decision.rs b/crates/cfc-daemon/src/decision.rs index 5f0cae6..d6670f7 100644 --- a/crates/cfc-daemon/src/decision.rs +++ b/crates/cfc-daemon/src/decision.rs @@ -44,33 +44,6 @@ struct EngineInner { on_change: RwLock>>, } -/// What [`Engine::process_wide_verdict`] found: the action that holds for a -/// process wherever it connects, and the rule that says so. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub struct ProcessWideVerdict { - pub action: cfc_core::Action, - pub rule_id: uuid::Uuid, - pub duration: cfc_core::Duration, -} - -impl ProcessWideVerdict { - /// Whether this verdict may be handed to the in-kernel fast path. - /// - /// An allow, from a rule that lasts. `Once` never reaches here (it is - /// not stored), and a timed rule is excluded on purpose: the fast path - /// re-checks grants only on flow starts and rule changes, so a timed - /// allow would keep marking sockets until the next flush noticed its - /// deadline - up to thirty seconds past the moment the user chose. - /// Denies never qualify either way; they have their own map. - pub fn fast_allow_eligible(&self) -> bool { - self.action == cfc_core::Action::Allow - && matches!( - self.duration, - cfc_core::Duration::Always | cfc_core::Duration::UntilRestart - ) - } -} - pub enum Decision { /// A persistent rule matched. Return the verdict immediately. Resolved(Verdict), @@ -137,14 +110,6 @@ impl Engine { self.notify_changed(); } - /// Credits a hit to `rule_id` for a flow the packet path never saw - a - /// fast-allowed connection reported by the kernel. The same counter - /// `evaluate` bumps, so the busiest allow rule stops reading as dead the - /// day its traffic skips the queue. - pub fn record_hit(&self, rule_id: uuid::Uuid) { - *self.inner.hits.lock().entry(rule_id).or_insert(0) += 1; - } - fn notify_changed(&self) { if let Some(f) = self.inner.on_change.read().as_ref() { f(); @@ -209,15 +174,6 @@ impl Engine { /// `None` means "ask the packet path", which is always a safe answer: it /// is what happened before this existed. pub fn process_wide_action(&self, proc: &Process) -> Option { - self.process_wide_verdict(proc).map(|v| v.action) - } - - /// [`process_wide_action`](Self::process_wide_action) with the rule that - /// answered: its id, for crediting a hit the packet path will never see, - /// and its duration, because the fast path is offered only to rules that - /// last. A timed allow (`--for 1h`) keeps the packet path, so its expiry - /// is exact rather than "within the next flush tick". - pub fn process_wide_verdict(&self, proc: &Process) -> Option { let now_unix_ms = chrono::Utc::now().timestamp_millis(); let rules = self.inner.rules.read(); for rule in rules @@ -247,43 +203,11 @@ impl Engine { if !rule.scope.matches_process(proc) { continue; } - return (!rule.scope.constrains_destination()).then_some(ProcessWideVerdict { - action: rule.action, - rule_id: rule.id, - duration: rule.duration, - }); + return (!rule.scope.constrains_destination()).then_some(rule.action); } None } - /// Whether any enabled rule could grant the fast path to *some* process. - /// - /// The same predicate `process_wide_verdict` applies per process, asked of - /// the rule set as a whole: outbound, not flow-scoped, and eligible - an - /// `Allow` that lasts. Reuses [`ProcessWideVerdict::fast_allow_eligible`] - /// rather than restating it, so there is one definition of "could grant". - /// - /// For `sweep_fast_allow`, which otherwise walks /proc on every rule - /// change to reach a conclusion this answers in a few comparisons. - pub fn any_fast_allow_rule(&self) -> bool { - let now_unix_ms = chrono::Utc::now().timestamp_millis(); - let rules = self.inner.rules.read(); - rules - .rules - .iter() - .filter(|r| r.enabled && !r.is_expired(now_unix_ms)) - .filter(|r| r.scope.direction != Some(cfc_core::Direction::Inbound)) - .filter(|r| !r.scope.constrains_destination()) - .any(|r| { - ProcessWideVerdict { - action: r.action, - rule_id: r.id, - duration: r.duration, - } - .fast_allow_eligible() - }) - } - /// Whether some resolution of the rules this caller cannot decide would /// still deny this process outright - the question that separates the two /// meanings of `process_wide_action`'s `None`. @@ -414,42 +338,6 @@ impl Engine { ) } - /// Whether any live rule scoped to a uid could apply to `exe`. - /// - /// The mirror of [`Self::compilable_exe_paths`], for the grant map rather - /// than the deny map, and it exists for the same reason turned around. - /// - /// The two deciders do not read the same uid. The packet path takes the - /// uid from the kernel's exec record when it has one - the uid at - /// `execve` - and does not read `/proc//status` at all. The grant - /// path resolves through `/proc` and gets the uid the process holds - /// *now*. For a program that drops privileges after exec - `named`, - /// `postfix`, a browser entering its sandbox - those are different, so - /// `deny --exe X --uid 0` above `allow --exe X` can be answered "deny" by - /// the packet path and "allow" by the grant path. A grant is process-wide - /// and destination-blind, so that disagreement is not a slower answer, it - /// is the deny never being applied at all. - /// - /// Rather than decide which uid is the right one - a semantic change to - /// what every existing uid-scoped rule means - the fast path simply - /// abstains wherever a uid could matter. Those flows take the queue, - /// where the uid question has one answer and it is the packet path's. - /// Hosts with no uid-scoped rule, which is nearly all of them, pay - /// nothing: the walk stops at the first predicate. - pub fn uid_scoped_may_apply(&self, exe: &std::path::Path) -> bool { - let now_unix_ms = chrono::Utc::now().timestamp_millis(); - self.inner - .rules - .read() - .rules - .iter() - .filter(|r| r.enabled && !r.is_expired(now_unix_ms)) - .filter(|r| r.scope.uid.is_some()) - // A uid-scoped rule that names no executable can apply to any - // program, exactly as in `compilable_exe_paths`. - .any(|r| r.scope.exe_path.as_deref().is_none_or(|p| p == exe)) - } - /// What an inbound flow gets when no rule matches. /// /// Separate from `no_ui_action` because it answers a different question. @@ -665,83 +553,6 @@ mod tests { Engine::new(RuleSet { rules }, shared(dp_deny())) } - // --- the uid guard on the grant path -------------------------------- - - #[test] - fn a_uid_scoped_rule_takes_its_program_off_the_fast_path() { - let exe = std::path::Path::new("/usr/sbin/named"); - let other = std::path::Path::new("/usr/bin/curl"); - - // The shape that made the fast path grant what the packet path denies: - // the program execs as root and drops to its own uid, so the grant - // path reads 53 and the packet path reads 0, and only one of them - // sees the deny. - let mut denied_as_root = RuleScope::any(); - denied_as_root.exe_path = Some(PathBuf::from(exe)); - denied_as_root.uid = Some(0); - let mut allowed = RuleScope::any(); - allowed.exe_path = Some(PathBuf::from(exe)); - let engine = engine_with(vec![ - Rule::new("deny-as-root".to_string(), Action::Deny, denied_as_root), - Rule::new("allow".to_string(), Action::Allow, allowed), - ]); - assert!( - engine.uid_scoped_may_apply(exe), - "a uid-scoped rule names this program, so the fast path must stand aside" - ); - assert!( - !engine.uid_scoped_may_apply(other), - "a program no uid-scoped rule names is unaffected" - ); - - // A rule set with no uid predicate at all costs nothing: this is the - // common case and it must not be taken off the fast path. - let mut plain = RuleScope::any(); - plain.exe_path = Some(PathBuf::from(exe)); - let engine = engine_with(vec![Rule::new("a".to_string(), Action::Allow, plain)]); - assert!(!engine.uid_scoped_may_apply(exe)); - } - - #[test] - fn a_uid_rule_naming_no_program_takes_everything_off_the_fast_path() { - // It could apply to anything, and nothing here can tell whether it - // would - the same reasoning `compilable_exe_paths` uses to return - // `None` rather than a list. - let mut any_program = RuleScope::any(); - any_program.uid = Some(1000); - let engine = engine_with(vec![Rule::new( - "per-user".to_string(), - Action::Deny, - any_program, - )]); - for exe in ["/usr/bin/curl", "/usr/sbin/named", "/opt/whatever"] { - assert!( - engine.uid_scoped_may_apply(std::path::Path::new(exe)), - "{exe} must not be granted while a uid rule can reach any program" - ); - } - } - - #[test] - fn a_disabled_or_expired_uid_rule_does_not_hold_the_fast_path_back() { - let exe = std::path::Path::new("/usr/sbin/named"); - let mut scope = RuleScope::any(); - scope.exe_path = Some(PathBuf::from(exe)); - scope.uid = Some(0); - - let mut disabled = Rule::new("off".to_string(), Action::Deny, scope.clone()); - disabled.enabled = false; - assert!(!engine_with(vec![disabled]).uid_scoped_may_apply(exe)); - - let mut expired = Rule::new("gone".to_string(), Action::Deny, scope); - expired.duration = cfc_core::Duration::Seconds(1); - expired.created_at = chrono::Utc::now() - chrono::Duration::hours(1); - assert!( - !engine_with(vec![expired]).uid_scoped_may_apply(exe), - "an expired rule constrains nothing" - ); - } - // --- process_wide_action ------------------------------------------- // // This is what the `cgroup/connect4|6` programs are steered by, and it @@ -869,186 +680,29 @@ mod tests { assert_eq!(engine.process_wide_action(&without), None); } - /// The eligibility predicate, reached the way the daemon reaches it: through - /// `process_wide_verdict` on a real engine with a real rule, one case per - /// shape of answer. + /// A rule that says anything about the flow must never become a + /// process-wide answer. /// - /// The exhaustive version further down tests the predicate on its own over - /// the whole Action x Duration space; this one is the through-the-engine - /// complement, and the pair is deliberate. (The doc comment that used to - /// sit here described a rule-expiry test that lives elsewhere - a leftover - /// from a move, and a reader looking for the expiry test would have been - /// sent to the wrong function.) - #[test] - fn only_lasting_allows_are_fast_allow_eligible() { - // The fast path re-checks grants on flow starts and rule changes, not - // on a clock, so a timed allow would outlive its deadline by up to a - // flush tick. Denies have their own map and never qualify. - let mut scope = RuleScope::any(); - scope.exe_path = Some(PathBuf::from("/usr/bin/curl")); - let eligible = |action: Action, duration: cfc_core::Duration| { - let mut rule = Rule::new("r".to_string(), action, scope.clone()); - rule.duration = duration; - let engine = engine_with(vec![rule]); - let proc = Process { - exe: PathBuf::from("/usr/bin/curl"), - ..Process::unknown(1) - }; - engine - .process_wide_verdict(&proc) - .map(|v| v.fast_allow_eligible()) - }; - assert_eq!( - eligible(Action::Allow, cfc_core::Duration::Always), - Some(true) - ); - assert_eq!( - eligible(Action::Allow, cfc_core::Duration::UntilRestart), - Some(true) - ); - assert_eq!( - eligible(Action::Allow, cfc_core::Duration::Seconds(3600)), - Some(false), - "a timed allow keeps the packet path so its expiry is exact" - ); - assert_eq!( - eligible(Action::Deny, cfc_core::Duration::Always), - Some(false) - ); - assert_eq!( - eligible(Action::Reject, cfc_core::Duration::Always), - Some(false) - ); - } - - /// The rule-set-wide question `sweep_fast_allow` asks before walking /proc: - /// one case per way a rule set can fail to grant anyone, and one that can. + /// `deny --dst-port 443` means "this program may not reach 443", and an + /// in-kernel verdict means "every `connect()` this program makes is + /// refused". Conflating them would refuse everything the rule never + /// named, so each predicate that makes a rule flow-scoped is checked on + /// its own - a missing one would only show up as a rule type that quietly + /// denies everything. #[test] - fn a_rule_set_that_cannot_grant_anyone_says_so() { - use cfc_core::Duration; - let exe = || Some(PathBuf::from("/usr/bin/curl")); - let rule = |action: Action, duration: Duration, f: fn(&mut RuleScope)| { - let mut scope = RuleScope::any(); - scope.exe_path = exe(); - f(&mut scope); - let mut r = Rule::new("r".to_string(), action, scope); - r.duration = duration; - r - }; - let none = |_: &mut RuleScope| {}; - - assert!(!engine_with(vec![]).any_fast_allow_rule(), "no rules"); - assert!( - !engine_with(vec![rule(Action::Deny, Duration::Always, none)]).any_fast_allow_rule(), - "only denies" - ); - assert!( - !engine_with(vec![rule(Action::Allow, Duration::Seconds(3600), none)]) - .any_fast_allow_rule(), - "only a timed allow" - ); - assert!( - !engine_with(vec![rule(Action::Allow, Duration::Always, |s| s - .dst_port = - Some(443))]) - .any_fast_allow_rule(), - "only a flow-scoped allow" - ); - let mut disabled = rule(Action::Allow, Duration::Always, none); - disabled.enabled = false; - assert!( - !engine_with(vec![disabled]).any_fast_allow_rule(), - "only a disabled allow" - ); - - assert!( - engine_with(vec![ - rule(Action::Deny, Duration::Always, none), - rule(Action::Allow, Duration::Always, none), - ]) - .any_fast_allow_rule(), - "one lasting outright allow among denies is enough" - ); - } - - /// The one predicate the fast path's safety rests on, over its whole - /// input space rather than a sample: three actions and four durations is - /// all of it. - /// - /// The expected answers are a table, not the implementation's own - /// expression rewritten - a test that recomputes what it is checking - /// passes for a wrong implementation too. Adding a variant to either enum - /// breaks this test, which is the point: a new action or a new duration is - /// a decision about whether it may mark a socket, and it should not be - /// possible to make it by accident. - #[test] - fn only_a_lasting_allow_may_ever_mark_a_socket() { - use cfc_core::{Action, Duration}; - - // Everything that may. Everything else may not. - let may_mark = [ - (Action::Allow, Duration::Always), - (Action::Allow, Duration::UntilRestart), - ]; - - let every_action = [Action::Allow, Action::Deny, Action::Reject]; - let every_duration = [ - Duration::Once, - Duration::UntilRestart, - Duration::Always, - // Both ends of the timed range: a timed allow must never mark, - // because the mark outlives the second it expires on - nothing - // re-passes a hook just because a clock ticked. - Duration::Seconds(0), - Duration::Seconds(u32::MAX), - ]; - - for action in every_action { - for duration in every_duration { - let verdict = ProcessWideVerdict { - action, - rule_id: uuid::Uuid::nil(), - duration, - }; - let expected = may_mark.contains(&(action, duration)); - assert_eq!( - verdict.fast_allow_eligible(), - expected, - "{action:?} + {duration:?} must {} be fast-allow eligible", - if expected { "" } else { "not" } - ); - } - } - } - - /// A rule that says anything about the flow must never become a blanket - /// mark on a process's sockets. - /// - /// `allow --dst-port 443` means "this program may reach 443", and a mark - /// means "every packet this program sends skips the queue". Conflating - /// them would be the widest possible failure of this feature, so each - /// predicate that makes a rule flow-scoped is checked on its own - a - /// missing one would only show up as a rule type that quietly grants - /// everything. - #[test] - fn a_flow_scoped_allow_never_grants_the_fast_path() { + fn a_flow_scoped_rule_never_yields_a_process_wide_action() { let exe = PathBuf::from("/usr/bin/curl"); let proc = Process { exe: exe.clone(), ..Process::unknown(1) }; - // The unconstrained rule does grant - otherwise the cases below would + // The unconstrained rule does answer - otherwise the cases below would // pass for the wrong reason. let mut open = RuleScope::any(); open.exe_path = Some(exe.clone()); - let engine = engine_with(vec![Rule::new("open".to_string(), Action::Allow, open)]); - assert!( - engine - .process_wide_verdict(&proc) - .is_some_and(|v| v.fast_allow_eligible()), - "an exe-only allow is the case this feature exists for" - ); + let engine = engine_with(vec![Rule::new("open".to_string(), Action::Deny, open)]); + assert_eq!(engine.process_wide_action(&proc), Some(Action::Deny)); // Named, because clippy is right that the bare tuple is a mouthful - // and because the name says what the table is: one way each of making @@ -1074,36 +728,15 @@ mod tests { let mut scope = RuleScope::any(); scope.exe_path = Some(exe.clone()); constrain(&mut scope); - let engine = engine_with(vec![Rule::new(what.to_string(), Action::Allow, scope)]); - let granted = engine - .process_wide_verdict(&proc) - .is_some_and(|v| v.fast_allow_eligible()); - assert!( - !granted, - "an allow scoped by {what} must not mark every socket this process opens" + let engine = engine_with(vec![Rule::new(what.to_string(), Action::Deny, scope)]); + assert_eq!( + engine.process_wide_action(&proc), + None, + "a deny scoped by {what} must not refuse every connect() this process makes" ); } } - #[test] - fn process_wide_verdict_names_the_rule_that_answered() { - // The allow consumer credits hits by this id; the wrong id would - // credit the wrong rule, which is worse than crediting none. - let mut scope = RuleScope::any(); - scope.exe_path = Some(PathBuf::from("/usr/bin/curl")); - let rule = Rule::new("mine".to_string(), Action::Allow, scope); - let id = rule.id; - let engine = engine_with(vec![rule]); - let proc = Process { - exe: PathBuf::from("/usr/bin/curl"), - ..Process::unknown(1) - }; - let v = engine.process_wide_verdict(&proc).expect("a verdict"); - assert_eq!(v.rule_id, id); - assert_eq!(v.action, Action::Allow); - assert_eq!(engine.process_wide_action(&proc), Some(Action::Allow)); - } - /// An expired rule must drop out of what gets compiled into the kernel. /// /// The kernel table is rebuilt on rule edits, and expiry is not one. The @@ -1522,7 +1155,7 @@ mod tests { let original = allow_port_rule(80); let id = original.id; let engine = engine_with(vec![original]); - engine.record_hit(id); + engine.evaluate(&conn(80), &proc("/usr/bin/curl")); let mut batch = engine.snapshot().rules; batch.push(allow_port_rule(443)); engine.replace_rules(batch); diff --git a/crates/cfc-daemon/src/ebpf.rs b/crates/cfc-daemon/src/ebpf.rs index 4a40a0e..e7a50e9 100644 --- a/crates/cfc-daemon/src/ebpf.rs +++ b/crates/cfc-daemon/src/ebpf.rs @@ -374,131 +374,6 @@ pub fn enforcement_level() -> Option { } } -/// Whether the fast-allow path - a process-wide allow marking its sockets so -/// nftables accepts them ahead of the queue - is actually doing anything. -/// -/// Two-way, with the reason attached. `Off` carries *why*, because the path -/// has many ways to be silently inert - the config switch, a kernel whose -/// verifier lacks `bpf_setsockopt` on sock_addr, exit not tracked at all, ring -/// consumers that did not start, an nftables set the snippet does not declare, -/// a table that is not loaded yet, a ruleset reload that emptied the set - and -/// a feature that is off for a reason nobody can read is -/// a feature nobody can rely on. That lesson was learned once already with the -/// enforcement level above. -/// -/// Not "an inherited attach from a build that predates it", which this list -/// used to open with. The pin directory carries the ABI version and the fast -/// path arrived with a version bump, so a daemon never inherits pins from a -/// build that lacks it - it would find no pins at that path at all and attach -/// fresh. -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum FastAllow { - /// Marks are being set and the nftables set holds this daemon's value. - /// - /// `deadline_secs` is how long a grant outlives a daemon that stops - /// refreshing it: the full [`fast_allow::DEADLINE_SECS`] when every - /// guarantee holds, the shorter [`fast_allow::DEADLINE_SECS_REDUCED`] when - /// one does not. `reduced` says which - the exec/exit links could not be - /// pinned, or exit is detected by leader only and grants are swept every - /// beat - or is `None` for the full guarantee. - /// - /// Carried rather than assumed, because a status line that says `live` - /// without saying which guarantee is a status line that misleads on - /// exactly the kernels where the guarantee is weaker. Two different - /// weaknesses give the same six seconds, so the number alone would not do. - Live { - deadline_secs: u64, - reduced: Option, - }, - /// Not doing anything, and this is the one sentence that says why. - Off(String), -} - -impl FastAllow { - /// One token plus the reason, for `cfc status`: `live` or `off: `. - pub fn describe(&self) -> String { - match self { - Self::Live { - reduced: None, - deadline_secs, - } if *deadline_secs == cfc_ebpf_common::fast_allow::DEADLINE_SECS => "live".to_string(), - Self::Live { - deadline_secs, - reduced, - } => match reduced { - Some(why) => format!("live, grants lapse within {deadline_secs}s ({why})"), - None => format!("live, grants lapse within {deadline_secs}s"), - }, - Self::Off(why) => format!("off: {why}"), - } - } -} - -static FAST_ALLOW_LEVEL: std::sync::RwLock> = std::sync::RwLock::new(None); - -/// Records the fast path's current state for `cfc status`. -/// -/// Not "called once": the loader publishes the startup decision before the -/// heartbeat exists, the heartbeat publishes every arm and every loss of the -/// nftables element, the late withdrawal publishes its refusal, and `Drop` -/// publishes the stop. Last writer wins, which is why the loader must publish -/// *before* spawning the heartbeat rather than after it returns. -pub fn set_fast_allow_level(level: FastAllow) { - *FAST_ALLOW_LEVEL - .write() - .unwrap_or_else(std::sync::PoisonError::into_inner) = Some(level); -} - -/// The fast path's state, `None` until startup has answered - the same -/// "ask again" sentinel `enforcement_level` uses, for the same reason. -pub fn fast_allow_level() -> Option { - FAST_ALLOW_LEVEL - .read() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .clone() -} - -/// What this kernel's verifier let the fast path have. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum FastPathCapability { - /// Cookie connect variants and both sendmsg programs verified. - Ready, - /// The connect hooks fell back to the `_basic` twins, which carry no - /// `mark_decision` at all: nothing would ever mark a socket, so the fast - /// path cannot run. In the same era of kernels, no `bpf_setsockopt` on - /// sock_addr either. - BasicConnect, - /// The connect hooks took with the mark decision in them, and a sendmsg - /// hook did not load, attach or pin. The path runs; this is a caveat. - /// - /// The likely cause is the verifier: this kernel allows `bpf_getsockopt` / - /// `bpf_setsockopt` on connect programs and not yet on UDP sendmsg ones - - /// 5.10 answers `unknown func bpf_getsockopt#57`, 6.12 accepts. It is not - /// the only cause, which is why neither this comment nor `caveat` states it - /// as fact: `attach_one` also fails at the attach, at taking the link, and - /// at pinning. The real error is in the log line beside it. - /// - /// Why this stopped refusing the path: the sendmsg hooks used to re-decide - /// a UDP socket's mark per datagram, and were load-bearing. No UDP socket - /// is marked any more, so all they can do is strip a mark somebody - /// *forged* onto an unconnected UDP socket - a process that was granted, - /// learned the value with `getsockopt`, was revoked, and set it back. - /// Defence in depth against a narrow attacker, worth having where the - /// kernel allows it and not worth the whole feature where it does not. - SendmsgUnavailable, -} - -impl FastPathCapability { - /// One grep-able word for the startup log line and the matrix summary. - pub fn as_str(self) -> &'static str { - match self { - Self::Ready => "ready", - Self::BasicConnect => "basic-connect", - Self::SendmsgUnavailable => "sendmsg-unavailable", - } - } -} - /// What actually came up. Reported once at startup and otherwise inert. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct Report { @@ -536,45 +411,20 @@ pub struct Report { /// more expensive" into something visible here, rather than into "it /// stopped loading on someone else's kernel". pub verified_insns: Vec<(String, u32)>, - /// The fast path's state after startup, `None` when the layer never got - /// far enough to have an opinion (eBPF off, object not loaded). - pub fast_allow: Option, /// Whether process exit is detected exactly (`group_dead` read from the - /// tracepoint record) rather than approximated by leader exit. A deny - /// evicted late is an inconvenience; a fast-allow grant evicted late is a - /// mark on a recycled pid - so when this is false the fast path runs with - /// the reduced deadline and sweeps its grants on every heartbeat, dropping - /// any pid whose start time no longer matches the one recorded at grant. - /// It used to refuse the path outright, which withheld it from every - /// kernel without `group_dead` - 5.10 and 6.12 in the matrix. + /// tracepoint record) rather than approximated by thread-group leader + /// exit. A kernel fact the matrix asserts; where it is false the exit + /// consumer confirms a leader exit against /proc before evicting. pub exit_precise: bool, /// Whether *both* lifecycle tracepoint links were pinned to bpffs, rather /// than merely attached. /// - /// The two are not the same and the difference is the fast path's whole - /// safety argument. `attach_tracepoint` re-attaches unpinned when a link - /// cannot be pinned - no `BPF_LINK_TYPE_PERF_EVENT` before 5.15, or a - /// read-only bpffs - which keeps eviction working for as long as this - /// daemon runs and stops the moment it does not. The connect programs' own - /// links are pinned separately and go on marking sockets either way, so - /// the difference is whether a grant can outlive this daemon at all - and - /// this flag is what tells the two cases apart. - /// - /// It is one of the two facts that select the reduced deadline rather than - /// gating the path - the other is `exit_precise` - and `cfc status` names - /// which of the two applies. For a day it was a rung on the eligibility - /// ladder instead. + /// `attach_tracepoint` re-attaches unpinned when a link cannot be pinned - + /// no `BPF_LINK_TYPE_PERF_EVENT` before 5.15, or a read-only bpffs - which + /// keeps eviction working for as long as this daemon runs and stops the + /// moment it does not, while the connect programs' own links stay pinned + /// and go on refusing. This flag is what tells the two cases apart. pub lifecycle_pinned: bool, - /// What this kernel's verifier let the fast path's kernel side have: - /// the cookie connect variants with both sendmsg hooks, the variants - /// without them, or only the `_basic` twins that mark nothing. The one - /// fact of the eligibility ladder that nothing else in this report - /// carries, and on a kernel older than 5.16 - which reports no verified - /// instruction counts - the only trace of which programs attached. `None` - /// where no connect hook attached at all: a layer that is off or could not - /// attach has no capability to report, and must not read as having the - /// `_basic` twins. - pub fast_path_capability: Option, } impl Report { @@ -705,19 +555,6 @@ impl Report { exit_tracking = self.exit_tracking, dns_capture = self.dns_capture, ppid_from_btf = self.ppid_offsets, - fast_path = self - .fast_path_capability - .map_or("none", FastPathCapability::as_str), - // The fast path has five reasons to be off and they used to reach - // `cfc status` only. An operator who set `fast_allow = true`, - // restarted, and never ran the CLI had no way to learn from the - // journal that the kernel had refused a hook - which is the whole - // point of degrading loudly. - fast_allow = self - .fast_allow - .as_ref() - .map(FastAllow::describe) - .unwrap_or_else(|| "not decided".to_string()), "attribution sources: sock_diag + /proc{}; hostnames: PTR + FCrDNS{}", if self.exec_tracking { " + eBPF exec events" @@ -754,18 +591,17 @@ pub fn nft_table_loaded() -> anyhow::Result { nft_set::table_loaded() } -/// Flushes a previous daemon's fast-allow mark out of the nftables set, for -/// the starts where [`start`] never reaches the loader's own flush: the layer -/// switched off in the config, or a build without it. The set outlives -/// daemons and the accept rule reads it whether or not anything still marks, -/// so a daemon that crashed while armed and came back with the layer off -/// would otherwise leave a standing bypass token behind it. A table that is -/// not loaded yet is not an error here - the nft unit is ordered after the -/// daemon - and `--dry-run` must not call this at all: it touches nothing, and -/// `main` is the one that knows it is running. -pub fn flush_stale_fast_allow() { - if let Err(e) = nft_set::disarm_for_start() { - tracing::error!("could not disable previous Fast Allow state: {e:#}; old marks may still bypass filtering; run systemctl reload colony-firewall-nft and inspect the journal before relying on filtering"); +/// Flushes the legacy `fast_allow` nftables set, once, at daemon start. +/// +/// Fast Allow shipped opt-in in 0.4.0 and is gone, but a 0.4-0.6 daemon that +/// crashed while armed can have left its mark in a set that a ruleset not yet +/// reloaded still accepts. Called from `main` in every build, whatever the +/// layer's mode, and never under `--dry-run`, which touches nothing. A table +/// or set that is not loaded is nothing to flush: at boot the nft unit is +/// ordered after the daemon. +pub fn flush_legacy_fast_allow_set() { + if let Err(e) = nft_set::flush() { + tracing::error!("could not flush the legacy fast_allow nftables set: {e:#}; a mark left by an older daemon may still bypass filtering; run systemctl reload colony-firewall-nft and inspect the journal before relying on filtering"); } } @@ -787,21 +623,10 @@ pub fn start( #[cfg_attr(not(feature = "ebpf"), allow(unused_variables))] engine: Option< crate::decision::Engine, >, - // The fast path's reporting: flows the kernel waved past the queue are - // fed back into the same observed stream and the same counters NFQUEUE - // feeds, so the live feed and the enforcing heuristic keep telling the - // truth about traffic the packet path never sees. - #[cfg_attr(not(feature = "ebpf"), allow(unused_variables))] - observed: tokio::sync::broadcast::Sender, - #[cfg_attr(not(feature = "ebpf"), allow(unused_variables))] stats: crate::stats::Stats, ) -> Runtime { - if cfg.fast_allow { - tracing::warn!("Fast Allow is disabled: socket marks cannot verify the current sender; use normal NFQUEUE filtering and remove fast_allow = true from daemon.toml"); + if cfg.fast_allow || cfg.fast_allow_mark.is_some() { + tracing::warn!("[ebpf] fast_allow and fast_allow_mark are ignored: Fast Allow was removed because a socket mark cannot prove which process sent a packet; delete them from daemon.toml"); } - // The answer until the layer says otherwise. Every early return below - // leaves it standing, which is the truthful default: no layer, no fast - // path. - set_fast_allow_level(FastAllow::Off("the in-kernel layer is not up".to_string())); if !cfg.enabled.wants_load() { return Runtime { report: Report::inert( @@ -825,9 +650,6 @@ pub fn start( // `dns` and `table` are the loader's inputs; without it they are // simply never wired to anything. let _ = (dns, table); - // And the loader's flush of a predecessor's mark is never reached in - // this build, so it happens here. - flush_stale_fast_allow(); Runtime { report: Report::inert_because( cfg.enabled, @@ -858,19 +680,7 @@ pub fn start( loader::Trust::Refuse, ), }; - match loader::load_and_attach( - &path, - dns, - table.clone(), - engine, - trust, - observed, - stats, - loader::FastAllowCfg { - on: cfg.fast_allow, - mark: cfg.fast_allow_mark, - }, - ) { + match loader::load_and_attach(&path, dns, table.clone(), engine, trust) { Ok((attached, mut report)) => { // The loader builds its report before it knows how it was // asked for; only `start` does. Without this an `auto` host @@ -878,33 +688,20 @@ pub fn start( // error policy. report.mode = cfg.enabled; table.set_live(report.exec_tracking); - // The fast-allow level is NOT published here. The loader - // publishes it before it spawns the heartbeat, because that - // task publishes too - and this call site runs after both, so - // it could only ever overwrite a fresher answer with a staler - // one. It did: on a restart the heartbeat arms immediately and - // says `Live`, and this line put "waiting for the nftables - // table" back on top of it, permanently. Runtime { report, _attached: Some(attached), } } - Err(e) => { - // The loader never got far enough to publish one. - set_fast_allow_level(FastAllow::Off( - "the in-kernel layer did not come up".to_string(), - )); - Runtime { - report: Report::inert_because( - cfg.enabled, - true, - e.degrade, - format!("load failed, continuing without it: {:#}", e.source), - ), - _attached: None, - } - } + Err(e) => Runtime { + report: Report::inert_because( + cfg.enabled, + true, + e.degrade, + format!("load failed, continuing without it: {:#}", e.source), + ), + _attached: None, + }, } }; @@ -928,8 +725,6 @@ mod tests { DnsCache::new(), proc_table::KernelProcTable::new(), None, - tokio::sync::broadcast::channel(8).0, - crate::stats::Stats::new(), ); assert!(!rt.report.any_active()); assert_eq!(rt.report.mode, EbpfMode::Off); @@ -962,14 +757,7 @@ mod tests { // what *this* load did, and against the process-wide table it would be // an assertion about every other test in the binary as well. let table = proc_table::KernelProcTable::new(); - let rt = start( - &cfg, - DnsCache::new(), - table.clone(), - None, - tokio::sync::broadcast::channel(8).0, - crate::stats::Stats::new(), - ); + let rt = start(&cfg, DnsCache::new(), table.clone(), None); assert_eq!(rt.report.mode, EbpfMode::On); assert!(!rt.report.any_active(), "nothing can have attached"); assert_eq!(rt.report.ring0(), Ring0::Unavailable); @@ -1023,14 +811,7 @@ mod tests { ); let table = proc_table::KernelProcTable::new(); - let rt = start( - &cfg, - DnsCache::new(), - table.clone(), - None, - tokio::sync::broadcast::channel(8).0, - crate::stats::Stats::new(), - ); + let rt = start(&cfg, DnsCache::new(), table.clone(), None); for note in &rt.report.notes { println!("note: {note}"); } diff --git a/crates/cfc-daemon/src/ebpf/enforce.rs b/crates/cfc-daemon/src/ebpf/enforce.rs index f19e456..364e6df 100644 --- a/crates/cfc-daemon/src/ebpf/enforce.rs +++ b/crates/cfc-daemon/src/ebpf/enforce.rs @@ -104,24 +104,21 @@ pub(super) const MAP_DENY_EVENTS: &str = "DENY_EVENTS"; pub(super) const MAP_EXE_RULES: &str = "EXE_RULES"; pub(super) const MAP_EXE_RULES_ON: &str = "EXE_RULES_ON"; -/// The fast path's maps. All four pinned, for the reason every enforcement -/// map is: a restarting daemon must steer the maps the still-attached -/// programs read, and an unpinned one would be a fresh map nobody reads. -/// Pinning is also what makes the deadline necessary - the programs keep -/// these alive after the daemon dies, so nothing empties `FAST_ALLOW` by -/// itself; `FAST_ALLOW_UNTIL` running out is what stops the marks. +/// The legacy Fast Allow maps. The daemon no longer grants, but the kernel +/// object still carries them (ABI v4) and the connect programs still read +/// them, so they stay pinned: [`disarm_legacy_fast_allow`] has to reach the +/// copies the pinned programs see, not a fresh map nobody reads. pub(super) const MAP_FAST_ALLOW: &str = "FAST_ALLOW"; pub(super) const MAP_FAST_ALLOW_UNTIL: &str = "FAST_ALLOW_UNTIL"; pub(super) const MAP_FAST_ALLOW_MARK: &str = "FAST_ALLOW_MARK"; pub(super) const MAP_ALLOW_EVENTS: &str = "ALLOW_EVENTS"; -/// The mark decision for UDP that never calls `connect()`. Fast-path only: -/// they refuse nothing, so a failure to attach them costs the fast path and -/// not enforcement, and they have no `_basic` twins. -pub(super) const PROG_SENDMSG4: &str = "cfc_sendmsg4"; -pub(super) const PROG_SENDMSG6: &str = "cfc_sendmsg6"; +/// Pin names a 0.4-0.6 daemon gave its Fast Allow sendmsg links, and the +/// directory it left beside the connect pins. Nothing attaches or reads these +/// any more; [`prepare`] removes them. pub(super) const LINK_SENDMSG4: &str = "sendmsg4"; pub(super) const LINK_SENDMSG6: &str = "sendmsg6"; +const LEGACY_COOKIE_MARKER: &str = "cookie-variants"; /// Pin name for the `sched_process_exit` link. /// @@ -182,58 +179,6 @@ pub(super) struct VerdictSink { /// What was last written to the kernel, so an unchanged recompute costs no /// syscalls. `None` until the first compile. last_compiled: Arc>>>, - /// The fast path's maps, `None` when this daemon must not grant: the - /// object predates them, or the loader judged the path ineligible (see - /// `FastAllow::Off`). Granting is gated here rather than at each call - /// site so an ineligible daemon cannot grant by accident from one path - /// and not another. - fast: Option, -} - -/// The kernel side of the fast path, from the daemon's chair. -/// -/// One rule for every writer here: **grants are re-earned, never inherited.** -/// The kernel clears `FAST_ALLOW` on exec and exit by itself; this side only -/// adds entries, and only for a process whose process-wide verdict is an -/// allow from a rule that lasts. Anything else - a deny, an abstention, a -/// destination-scoped rule, a timed allow, a process the engine cannot decide: -/// each of these *removes* the entry. There is no "keep" arm as there is for -/// denies, because a deny kept in doubt fails closed and an allow kept in -/// doubt is a bypass. -#[derive(Clone)] -pub(super) struct FastAllowMaps { - map: Arc>>, - until: Arc>>, - mark: Arc>>, - /// pid -> the rule that granted and the start time it was judged at. - /// - /// The rule is so an `ALLOW_EVENTS` record can credit the hit the packet - /// path will never see. The start time is what `sweep_stale_grants` compares - /// against, on a kernel whose exit detection is leader-only: a pid whose - /// start time no longer matches is not the process that was granted. - /// Userspace-only; a pid missing here when its event arrives is a grant - /// from a previous daemon, credited to nobody rather than to the wrong - /// rule. - granted_by: Arc>>, -} - -/// What the daemon remembers about one grant it wrote. -#[derive(Debug, Clone, Copy)] -struct Granted { - rule: uuid::Uuid, - /// `/proc//stat` field 22 when the grant was judged. `None` cannot - /// reach the map - `grant_if_still` refuses to grant without one - but the - /// type says so rather than the comment. - starttime: Option, -} - -/// One grant decision, for the three writers to share. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum Grant { - /// Write the entry; the rule that justifies it. - Yes(uuid::Uuid), - /// Remove the entry, whatever it held. - No, } impl VerdictSink { @@ -266,26 +211,6 @@ impl VerdictSink { ); } - // All three or none: a fast path with a grant map but no deadline map - // would be one the kernel honours forever, which is the exact state - // the deadline exists to make impossible. - let fast = match ( - bpf.take_map(MAP_FAST_ALLOW) - .and_then(|m| BpfHashMap::<_, u32, u32>::try_from(m).ok()), - bpf.take_map(MAP_FAST_ALLOW_UNTIL) - .and_then(|m| aya::maps::Array::<_, u64>::try_from(m).ok()), - bpf.take_map(MAP_FAST_ALLOW_MARK) - .and_then(|m| aya::maps::Array::<_, u32>::try_from(m).ok()), - ) { - (Some(map), Some(until), Some(mark)) => Some(FastAllowMaps { - map: Arc::new(Mutex::new(map)), - until: Arc::new(Mutex::new(until)), - mark: Arc::new(Mutex::new(mark)), - granted_by: Arc::new(Mutex::new(std::collections::HashMap::new())), - }), - _ => None, - }; - Ok(Self { map: Arc::new(Mutex::new(map)), engine, @@ -293,353 +218,9 @@ impl VerdictSink { exe_rules, exe_rules_on, last_compiled: Arc::new(Mutex::new(None)), - fast, }) } - /// The engine this sink decides with, for the allow consumer to credit - /// hits against. - pub(super) fn engine(&self) -> &Engine { - &self.engine - } - - /// Whether this sink can grant at all. The loader consults it to decide - /// the reported `FastAllow` state; it is true iff the maps exist. - pub(super) fn has_fast_path(&self) -> bool { - self.fast.is_some() - } - - /// Withdraws the ability to grant, for a daemon that loaded the maps but - /// then judged the path ineligible (config off, basic connect variants, - /// exit not tracked, could not arm) or lost it after arming (the ring - /// consumers did not start). Also unarms the kernel side and empties the - /// map, so every hook takes its cheapest exit and nothing is left to - /// honour whatever the deadline says. - pub(super) fn withdraw_fast_path(&mut self) { - if let Some(fast) = self.fast.take() { - // Unarm the kernel side too, not only empty the map. The maps are - // pinned, so a daemon that crashed while armed leaves - // `FAST_ALLOW_MARK` set; a successor started with the fast path - // *off* used to leave it that way, and every TCP `connect()` on the - // machine then paid a `getsockopt` and two map reads to strip a - // mark nobody would ever set - for as long as that daemon ran. With - // `UNARMED` written, `mark_decision` returns at its first array - // read, and the exec/exit programs skip their grant delete as well. - if let Err(e) = fast.until.lock().set(0, 0u64, 0) { - warn!("could not zero FAST_ALLOW_UNTIL while withdrawing the fast path: {e}"); - } - if let Err(e) = fast - .mark - .lock() - .set(0, cfc_ebpf_common::fast_allow::UNARMED, 0) - { - warn!("could not unarm FAST_ALLOW_MARK while withdrawing the fast path: {e}"); - } - let mut map = fast.map.lock(); - let pids: Vec = map.keys().flatten().collect(); - for pid in pids { - let _ = map.remove(&pid); - } - } - } - - /// The grant decision for one process: the shared rule for every writer. - fn grant_for(&self, proc: &Process) -> Grant { - // Abstain wherever a uid-scoped rule could apply. This decider and the - // packet path read different uids for a process that dropped - // privileges, and a grant is process-wide - see - // `Engine::uid_scoped_may_apply`, which explains why the answer is to - // step aside rather than to pick a uid. - if self.engine.uid_scoped_may_apply(&proc.exe) { - return Grant::No; - } - match self.engine.process_wide_verdict(proc) { - Some(v) if v.fast_allow_eligible() => Grant::Yes(v.rule_id), - _ => Grant::No, - } - } - - /// Applies a grant decision to the kernel map, under the caller's lock. - /// `judged_at` is the start time a `Yes` was decided against; a `No` does - /// not need one. - fn apply_grant( - fast: &FastAllowMaps, - map: &mut BpfHashMap, - pid: u32, - grant: Grant, - judged_at: Option, - ) { - match grant { - Grant::Yes(rule) => { - if let Err(e) = map.insert(pid, cfc_ebpf_common::fast_allow::GRANTED, 0) { - warn!(pid, "could not write a fast-allow grant: {e}"); - return; - } - fast.granted_by.lock().insert( - pid, - Granted { - rule, - starttime: judged_at, - }, - ); - } - Grant::No => { - // Absent is the common case and not an error: see `clear`. - let _ = clear(map, pid); - fast.granted_by.lock().remove(&pid); - } - } - } - - /// Recomputes the grant for one process, from any writer. - /// - /// `judged_at` is the start time the caller read `proc` at - see - /// [`grant_if_still`](Self::grant_if_still) for why a grant needs it and a - /// withdrawal does not. - fn regrant(&self, pid: u32, judged_at: Option, proc: &Process) { - self.grant_if_still(pid, proc, judged_at, self.grant_for(proc)); - } - - /// Withdraws any grant for `pid`, unconditionally. - /// - /// The counterpart to `grant_if_still`, and deliberately unguarded: - /// removing a grant from a pid whose owner changed is harmless, because - /// the new owner has not earned one yet and its own exec flow will grant - /// it if a rule says so. Withdrawing is always the safe direction. - fn drop_grant(&self, pid: u32) { - if let Some(fast) = self.fast.as_ref() { - Self::apply_grant(fast, &mut fast.map.lock(), pid, Grant::No, None); - } - } - - /// Applies a grant decision, but writes a *grant* only if `pid` still holds - /// the process the caller judged. - /// - /// Between the /proc read that produced the decision and this write there - /// is real work - the read itself, the engine call, and this lock - while - /// `on_exec` and the pinned exec program keep running. The pid can be - /// recycled in that window, and a grant landing on its new owner is a - /// marked socket that owner never earned: the fail-open direction, on a - /// process that may match no rule at all. - /// - /// The deny side has had this guard since the orphan sweep was written - - /// `doomed` carries the start time each pid was judged at - and the grant - /// side, which needs it more, did not have it. - fn grant_if_still(&self, pid: u32, judged: &Process, judged_at: Option, grant: Grant) { - let Some(fast) = self.fast.as_ref() else { - return; - }; - if !matches!(grant, Grant::Yes(_)) { - Self::apply_grant(fast, &mut fast.map.lock(), pid, grant, None); - return; - } - - // `None` is not a start time, it is the absence of a process, and - // comparing it directly let a grant through on exactly the case that - // must refuse. `proc_view` reads the exe, then the uid, then the start - // time; a process that exits in between yields a full view with - // `judged_at = None`, and `None != None` is false, so the guard fell - // through and wrote a grant for a pid with no process. Nothing would - // have cleared it either: the kernel's exec and exit clears both - // belong to a process that has already gone, and a pid recycled by a - // fork that never execs would inherit the mark. - let Some(judged_at) = judged_at else { - debug!(pid, "not granting: no start time, so no process to grant"); - return; - }; - if proc_starttime(pid) != Some(judged_at) { - debug!( - pid, - "not granting: the pid changed hands while it was judged" - ); - return; - } - - // Before the write, so an execve that has already happened is not paid - // for with a real marked flow. - if proc_exe(pid).as_deref() != Some(judged.exe.as_path()) { - debug!( - pid, - "not granting: the program changed while the grant was decided" - ); - return; - } - - Self::apply_grant(fast, &mut fast.map.lock(), pid, grant, Some(judged_at)); - - // And once more, on the program rather than the pid - after the write, - // deliberately. - // - // `execve` keeps the start time (field 22 of /proc//stat is when - // the *process* began, not when it last exec'd), so the guard above - // cannot see one. That matters because the kernel's exec program - // removes this pid's grant on every execve: a process judged as an - // allowed binary, exec'ing into a denied one while this function was - // deciding, would have its grant correctly cleared by the kernel and - // then reinstated here, for a program nothing granted. - // - // Checked before the write as well as after - and neither makes this - // race-free, which the comment here used to claim. - // - // What remains is the interval between the pre-check and the insert. - // An execve landing there has its grant cleared by the kernel and then - // re-added by this write, and the post-write check removes it again - // only after a `read_link`. A `connect()` inside *that* window finds - // the entry present and marks the socket, and removing the map entry - // afterwards does not unmark it. So the exposure is one flow rather - // than none. It is bounded, and no standing refusal is skipped - - // `VERDICTS` is consulted before `mark_decision` - but "narrower" is - // the honest word and "race-free" was not. - if proc_exe(pid).as_deref() != Some(judged.exe.as_path()) { - debug!( - pid, - "withdrawing: the program changed while the grant was decided" - ); - Self::apply_grant(fast, &mut fast.map.lock(), pid, Grant::No, None); - } - } - - /// The rule that granted `pid`, for crediting an `ALLOW_EVENTS` record. - pub(super) fn granted_by(&self, pid: u32) -> Option { - self.fast - .as_ref() - .and_then(|f| f.granted_by.lock().get(&pid).map(|g| g.rule)) - } - - /// Drops every grant whose pid no longer holds the process it was judged - /// for, and returns how many. - /// - /// For a kernel whose `sched_process_exit` record has no readable - /// `group_dead`. There the exit program evicts on thread-group *leader* - /// exit, so a process whose leader exits first and dies later is never - /// evicted - while this daemon is alive and refreshing the deadline, which - /// therefore bounds nothing. This is the bound instead: called on every - /// heartbeat, it compares each granted pid's current start time with the - /// one recorded when it was granted; a mismatch, or no process at all, is - /// a grant for whoever owns the pid next. The exposure that remains is a - /// pid recycled *and* connecting within one heartbeat, without an exec in - /// between (an exec clears the grant in the kernel). - /// - /// Snapshot first, then read /proc with no lock held: `on_exec` and - /// `on_exit` take `granted_by` from the ring consumers. - pub(super) fn sweep_stale_grants(&self) -> usize { - let Some(fast) = self.fast.as_ref() else { - return 0; - }; - let snapshot: Vec<(u32, Option)> = fast - .granted_by - .lock() - .iter() - .map(|(pid, g)| (*pid, g.starttime)) - .collect(); - let mut dropped = 0usize; - for (pid, recorded) in snapshot { - if !grant_is_stale(recorded, proc_starttime(pid)) { - continue; - } - // Re-read what is recorded *now*, not what the snapshot said. In - // the window since the snapshot this pid may have died, been - // recycled, exec'd, and been granted afresh by `on_exec` with its - // own start time - and dropping that grant on the strength of the - // old one would take a legitimate grant from a legitimate process - // until the next rule change. If the record moved, it is someone - // else's decision and stands. - let recorded_now = fast.granted_by.lock().get(&pid).map(|g| g.starttime); - if recorded_now != Some(recorded) { - continue; - } - self.drop_grant(pid); - dropped += 1; - } - dropped - } - - /// Empties `FAST_ALLOW`. At every start, before anything is granted: the - /// map is pinned, so it holds the previous daemon's grants, made under the - /// previous daemon's rules. Returns how many were dropped, for the log. - pub(super) fn flush_fast_allow(&self) -> usize { - let Some(fast) = self.fast.as_ref() else { - return 0; - }; - let mut map = fast.map.lock(); - let pids: Vec = map.keys().flatten().collect(); - let n = pids.len(); - for pid in pids { - let _ = map.remove(&pid); - } - fast.granted_by.lock().clear(); - n - } - - /// Writes the mark value the kernel side will set, arming the path. - /// - /// The deadline is zeroed first, and by this function rather than by - /// assumption. Callers used to say "the deadline is still zero until the - /// first `beat`, so nothing is honoured before the heartbeat runs", which - /// is not a property the code had: `FAST_ALLOW_UNTIL` is a *pinned* map, - /// so after an unclean death it holds whatever deadline the previous - /// daemon last wrote - up to a minute into the future. Nothing was - /// actually honoured on the strength of it, because the grant map is - /// flushed at start and the nft set holds no mark yet, but the sentence - /// was load-bearing in two comments and true in neither. One `set` makes - /// it true. - /// - /// Order matters: zero the deadline, then write the mark. Between the two - /// the kernel reads an armed mark against a lapsed deadline, counts a - /// `STALE`, and marks nothing. - pub(super) fn arm(&self, mark: u32) -> anyhow::Result<()> { - let fast = self - .fast - .as_ref() - .ok_or_else(|| anyhow!("no fast path to arm"))?; - fast.until - .lock() - .set(0, 0u64, 0) - .context("zeroing FAST_ALLOW_UNTIL")?; - fast.mark - .lock() - .set(0, mark, 0) - .context("writing FAST_ALLOW_MARK")?; - Ok(()) - } - - /// Pushes the deadline out to now + `deadline_secs` on `CLOCK_BOOTTIME`, - /// the clock `bpf_ktime_get_boot_ns` reads. Called every `HEARTBEAT_SECS` - /// by the runtime; if it ever stops being called, the kernel side stops - /// honouring grants within one deadline - by design, not by accident. - pub(super) fn beat(&self, deadline_secs: u64) -> anyhow::Result<()> { - let Some(fast) = self.fast.as_ref() else { - return Ok(()); - }; - let until = boottime_ns()? + deadline_secs * 1_000_000_000; - fast.until - .lock() - .set(0, until, 0) - .context("writing FAST_ALLOW_UNTIL")?; - Ok(()) - } - - /// Disarms immediately: zero deadline, unarmed mark, empty map. For a - /// clean shutdown, so the marks stop now rather than when the deadline - /// this daemon last wrote runs out. Best effort in every step - a daemon on its way out has - /// nowhere to report a failure but the log. - pub(super) fn disarm(&self) { - let Some(fast) = self.fast.as_ref() else { - return; - }; - if let Err(e) = fast.until.lock().set(0, 0u64, 0) { - warn!("could not zero FAST_ALLOW_UNTIL on shutdown: {e}"); - } - if let Err(e) = fast - .mark - .lock() - .set(0, cfc_ebpf_common::fast_allow::UNARMED, 0) - { - warn!("could not unarm FAST_ALLOW_MARK on shutdown: {e}"); - } - self.flush_fast_allow(); - } - /// Recomputes every live process's verdict. /// /// Called when the rule set changes, which is the only event that can @@ -655,12 +236,11 @@ impl VerdictSink { /// (`source="rule"`) and the kernel counters never moved. /// /// Cost, since it is no longer only map operations: a handful of small - /// /proc reads per process - three for the view, one to re-date it before - /// the deny write, and up to three more inside `grant_if_still` when the - /// answer is a grant - for every recently-exec'd process and every orphan, - /// and one walk of /proc for `sweep_fast_allow` at the end. All of it on whichever - /// thread changed the rules - an IPC handler or startup, never the packet - /// path - and only when a human or the CLI actually changed something. + /// /proc reads per process - three for the view and one to re-date it + /// before the deny write - for every recently-exec'd process and every + /// orphan. All of it on whichever thread changed the rules - an IPC + /// handler or startup, never the packet path - and only when a human or + /// the CLI actually changed something. /// /// None of those reads happen while a map lock is held. That is a /// constraint, not an accident: `on_exec` and `on_exit` block on the @@ -698,43 +278,18 @@ impl VerdictSink { .iter() .filter_map(|proc| { // From /proc, like every other decider - not from the exec - // record. + // record. The record carries the execve *string*, which is + // neither absolute for `./foo` nor resolved through a + // symlink, and the uid at exec, which a process that dropped + // privileges no longer holds; the orphan sweep below reads + // /proc, and two deciders that disagree about one process is + // the defect. // - // Both loops in this function ask one question about one - // process, and for a long time they asked it of different - // inputs: this one of the `ExecEvent` (the execve *string*, - // and the uid the process had when it exec'd), the orphan - // sweep below of /proc. Two deciders that disagree about the - // same process is the defect, and all three ways it showed up - // were fail-open: - // - // * `execve("./foo")` records no absolute path, so - // `absolute_exe` answered None and this loop skipped the pid - // whole. Deleting the rule that granted such a process, or - // replacing it with a Block, left the grant standing in the - // kernel: a marked socket past the queue for a program no - // rule allowed any more. - // * the execve string is what the caller typed, not what ran. - // A rule naming a symlink - or `/bin/curl` on a merged-usr - // system - granted here what `on_exec` and the packet path, - // both of which resolve, refuse. - // * the recorded uid is the uid at exec. A process that - // dropped privileges kept a uid-scoped grant it had stopped - // qualifying for until its next execve, and absent one, - // forever. - match proc_view(proc.pid) { - Some((view, judged_at)) => Some((proc.pid, view, judged_at)), - None => { - // Gone, or /proc unreadable. Do not fall back to the - // exec record - that is the guess this comment exists - // to refuse. Withdraw the grant (a grant kept in doubt - // is a marked socket) and leave the deny to the exit - // tracepoint, which owns eviction and can tell - // "exited" from "unreadable". - self.drop_grant(proc.pid); - None - } - } + // Gone, or /proc unreadable: no fallback to the exec record, + // which is the guess this comment exists to refuse. The deny + // is left to the exit tracepoint, which owns eviction and can + // tell "exited" from "unreadable". + proc_view(proc.pid).map(|(view, judged_at)| (proc.pid, view, judged_at)) }) .collect(); @@ -800,18 +355,6 @@ impl VerdictSink { } drop(map); - // The grant side, with the verdict lock released: `grant_if_still` - // takes the grant map's own mutex, and holding both at once would put - // an ordering constraint on two locks that otherwise never nest. - // - // No tri-state here: the same engine answer either says "allow, - // lasting" or the entry goes. In particular an abstention - which - // keeps a deny above - removes a grant, because a grant kept in doubt - // is a marked socket past the queue. - for (pid, as_process, judged_at) in &views { - self.grant_if_still(*pid, as_process, *judged_at, self.grant_for(as_process)); - } - // And the entries the live list does not cover. // // `live_processes` only returns pids the proc table has seen exec @@ -829,31 +372,12 @@ impl VerdictSink { // dropped record, which is a process with no in-kernel verdict. let known: std::collections::HashSet = live.iter().map(|p| p.pid).collect(); let map = self.map.lock(); - let mut orphans: Vec = map + let orphans: Vec = map .keys() .flatten() .filter(|pid| !known.contains(pid)) .collect(); drop(map); - // Grants have orphans too - a long-running allowed process drops off - // the live list on the same TTL - and a grant whose rule is gone must - // go with it. Walk the grant map's own keys; the loop below re-decides - // each pid from /proc and applies the grant answer alongside the deny - // answer, so the two maps never disagree about one process. - if let Some(fast) = self.fast.as_ref() { - let granted: Vec = fast - .map - .lock() - .keys() - .flatten() - .filter(|pid| !known.contains(pid)) - .collect(); - for pid in granted { - if !orphans.contains(&pid) { - orphans.push(pid); - } - } - } // Each doomed pid carries the start time it was judged at, so the // final pass can tell "still the process I judged" from "the kernel @@ -870,10 +394,8 @@ impl VerdictSink { // a uid-scoped allow - so guessing would clear a denial the allow // was never meant to lift, which is the fail-open direction. let Some((proc, judged_at)) = proc_view(pid) else { - // Gone. Clear, or a recycled pid inherits its answer - and a - // grant even more so. + // Gone. Clear, or a recycled pid inherits its answer. doomed.push((pid, None)); - self.drop_grant(pid); continue; }; // `None` is two opposite answers and they must not be conflated. @@ -895,12 +417,6 @@ impl VerdictSink { } } } - // The grant answer for the same process, from the same /proc - // read - with the real uid, so a process that dropped privileges - // loses a uid-scoped grant here rather than keeping what it - // earned as root - and against the same start time, so it cannot - // land on a pid the kernel recycled while this loop was working. - self.grant_if_still(pid, &proc, judged_at, self.grant_for(&proc)); } if !doomed.is_empty() { @@ -935,80 +451,6 @@ impl VerdictSink { processes = live.len(), denied, "resynced in-kernel verdicts after a rule change" ); - - // And the processes neither loop above can reach. - self.sweep_fast_allow(); - } - - /// Grants every process on the machine that a lasting rule allows. - /// - /// Every other writer of the grant map needs an *event*: `on_exec` needs an - /// execve, and the two loops above walk the proc table's recent execs and - /// the maps' own keys. None of them reaches a process that was already - /// running - which is exactly the population this feature exists for. It - /// showed up two ways, and in both the path reported `live` while doing - /// nothing at all: - /// - /// * after `systemctl restart colony-firewalld`. The pinned map is flushed - /// at start (those grants were made under the previous daemon's rules) - /// and the proc table starts empty, so the browser, the mail client - - /// everything long-lived - was never granted again for the rest of that - /// daemon's life. The restart is the common case: an upgrade, a crash, a - /// config reload. - /// * `allow --exe .../firefox always` on a browser started three hours ago. - /// The proc table's entries expire on a one-hour TTL, so the live loop - /// never saw it either. The feature only ever worked for a process that - /// exec'd *after* the daemon and less than an hour before its rule. - /// - /// So this walks /proc. O(processes), three small reads each and up to - /// three more for the ones a rule grants, on whichever thread changed the - /// rules - an IPC handler or startup, never - /// the packet path - and rule changes are paced by a human or the CLI. - /// - /// It only ever *adds*. Withdrawal is already covered and must stay where - /// it is: every pid holding a grant is re-decided by the live loop or the - /// orphan sweep above, and those two also handle pids that have left /proc - /// entirely, which this walk by construction cannot see. - pub(super) fn sweep_fast_allow(&self) { - if self.fast.is_none() { - return; - } - // A rule set that cannot grant anyone - only denies, only timed or - // flow-scoped allows - makes the walk below a few hundred /proc reads - // and engine calls for an answer already known. That is the common - // shape of a rule set with the fast path switched on, and this runs on - // every rule change. - if !self.engine.any_fast_allow_rule() { - debug!("no rule could grant the fast path; not walking /proc"); - return; - } - let entries = match std::fs::read_dir("/proc") { - Ok(e) => e, - Err(e) => { - warn!("could not read /proc to seed fast-allow grants: {e}"); - return; - } - }; - let (mut seen, mut granted) = (0usize, 0usize); - for entry in entries.flatten() { - let Some(pid) = entry - .file_name() - .to_str() - .and_then(|s| s.parse::().ok()) - else { - continue; - }; - seen += 1; - let Some((proc, judged_at)) = proc_view(pid) else { - continue; - }; - let grant = self.grant_for(&proc); - if matches!(grant, Grant::Yes(_)) { - granted += 1; - self.grant_if_still(pid, &proc, judged_at, grant); - } - } - debug!(seen, granted, "swept /proc for fast-allow grants"); } /// Decides whether this newly-exec'd process gets an in-kernel answer. @@ -1017,14 +459,10 @@ impl VerdictSink { /// process depends on a destination. Two things follow from that, both /// deliberate: /// - /// * **an allow is never written *here*.** `VERDICTS` holds denials only. - /// Allows that buy something - a lasting, process-wide one - go to the - /// fast-allow map through [`regrant`](Self::regrant) at the end of this - /// function, under rules of their own: cleared by the kernel on exec and - /// exit, honoured only while the daemon's heartbeat keeps the deadline - /// ahead of now, and re-earned per execve. A stale allow after pid reuse - /// is a security problem rather than an inconvenience, which is why the - /// two maps do not share a sweep. + /// * **an allow is never written.** `VERDICTS` holds denials only; an + /// allowed process keeps the packet path, which re-checks every flow. A + /// stale allow after pid reuse would be a security problem rather than an + /// inconvenience, which is why there is none to go stale. /// * **a stale entry is always cleared**, even when the answer is "no /// answer". A pid that re-execs into a different binary must not inherit /// the verdict written for the one before it. @@ -1053,8 +491,6 @@ impl VerdictSink { None => exe, } }); - // Dates the read above, for the grant at the end of this function. - let judged_at = resolved.is_some().then(|| proc_starttime(pid)).flatten(); // The uid here stays the event's, not a fresh read: at the moment of an // execve that *is* the process's uid, and a drop of privileges between // the kernel's tracepoint and this consumer is both vanishingly narrow @@ -1062,10 +498,9 @@ impl VerdictSink { // both. Mixing a live path with an event-time uid is worth naming // rather than leaving for a reader to find. // When /proc is unreadable, retain the unknown executable supplied by - // the event consumer. The execve argument cannot attest a mapped image. - // Missing identity keeps grants absent and leaves packet policy to - // NFQUEUE; it must not satisfy an executable-scoped rule. - let readable = resolved.is_some(); + // the event consumer. The execve argument cannot attest a mapped image, + // so missing identity leaves packet policy to NFQUEUE; it must not + // satisfy an executable-scoped rule. let corrected = match resolved { Some(exe) if exe != proc.exe => Some(Process { exe, @@ -1097,22 +532,6 @@ impl VerdictSink { debug!(pid, exe = %as_process.exe.display(), "in-kernel deny installed"); } drop(map); - - // The grant, re-earned for this exec. The kernel already removed the - // predecessor's entry on the exec path, so this is the daemon's only - // role in the fast path: say yes for the new binary, or say nothing. - // The engine is asked once more rather than reusing `deny` because - // the answer that matters here is "allow, from a rule that lasts", - // which the deny decision above did not compute. - if readable { - self.regrant(pid, judged_at, as_process); - } else { - // Nothing to grant for a pid we could not read. The kernel's exec - // path already removed the predecessor's entry, so this only - // clears the daemon-side bookkeeping that would otherwise credit - // an allow event to a rule for a process that no longer exists. - self.drop_grant(pid); - } } /// Compiles the process-wide rules into the kernel's own table. @@ -1226,54 +645,10 @@ impl VerdictSink { if let Err(e) = clear(&mut self.map.lock(), pid) { warn!(pid, "could not evict the in-kernel verdict: {e}"); } - // The kernel evicted the grant itself on the exit path; this drops - // the credit record so a recycled pid is never credited to a rule - // that granted its predecessor. - if let Some(fast) = self.fast.as_ref() { - let _ = clear(&mut fast.map.lock(), pid); - fast.granted_by.lock().remove(&pid); - } - } -} - -/// `CLOCK_BOOTTIME` in nanoseconds - the clock `bpf_ktime_get_boot_ns` -/// reads, which counts through suspend. The deadline it feeds must be sixty -/// wall-clock seconds, not sixty awake ones. -fn boottime_ns() -> anyhow::Result { - let mut ts = libc::timespec { - tv_sec: 0, - tv_nsec: 0, - }; - // SAFETY: a valid pointer to a timespec on our own stack; the call writes - // it and nothing else. - let rc = unsafe { libc::clock_gettime(libc::CLOCK_BOOTTIME, &mut ts) }; - if rc != 0 { - return Err(std::io::Error::last_os_error()).context("clock_gettime(CLOCK_BOOTTIME)"); - } - Ok(u64::try_from(ts.tv_sec).unwrap_or(0) * 1_000_000_000 - + u64::try_from(ts.tv_nsec).unwrap_or(0)) -} - -/// Whether a grant judged at `recorded` still belongs to the process now -/// holding its pid, given the start time read `now`. -/// -/// Only an exact match keeps a grant. `None` on either side is "no process": -/// a grant recorded without a start time cannot happen (`grant_if_still` -/// refuses it) but is treated as stale rather than trusted, and a pid with no -/// process now is a grant for whoever gets the pid next. Two live processes -/// never share a (pid, start time) pair within a boot. -fn grant_is_stale(recorded: Option, now: Option) -> bool { - match (recorded, now) { - (Some(a), Some(b)) => a != b, - _ => true, } } /// `/proc//exe`, with the kernel's `" (deleted)"` suffix stripped. -/// -/// One reader, because three callers want the same normalisation and two of -/// them are a guard and its counter-check - a difference between those two -/// would be a grant kept or withdrawn for a reason nobody wrote down. fn proc_exe(pid: u32) -> Option { let exe = std::fs::read_link(format!("/proc/{pid}/exe")).ok()?; let s = exe.to_string_lossy(); @@ -1295,7 +670,7 @@ fn proc_exe(pid: u32) -> Option { /// abstain and keeps the tri-state the sweep depends on. /// /// `None` means the process is gone or its /proc is unreadable, which callers -/// must treat as "no grant" rather than falling back to a guess. +/// must treat as "no answer" rather than falling back to a guess. fn proc_view(pid: u32) -> Option<(Process, Option)> { // `proc_exe` strips the kernel's " (deleted)" suffix - a package upgrade // under a running program, which process_resolve calls Tuesday on a rolling @@ -1351,9 +726,9 @@ fn proc_starttime(pid: u32) -> Option { // Same reason as `proc_uid`, and it matters more here: the kernel writes // comm unescaped into this file, and every guard built on this function // treats `None` as "no process". A program named with non-UTF-8 bytes was - // therefore never granted the fast path, and - once the deny pass started - // filtering on the start time - never given an in-kernel deny either, - // which is resync's whole job. Permanently, not transiently. + // therefore - once the deny pass started filtering on the start time - + // never given an in-kernel deny, which is resync's whole job. + // Permanently, not transiently. let stat = String::from_utf8_lossy(&std::fs::read(format!("/proc/{pid}/stat")).ok()?).into_owned(); // The comm field is parenthesised and may itself contain spaces and @@ -1399,22 +774,17 @@ fn is_absent(err: &aya::maps::MapError) -> bool { } /// Per-CPU counters, summed. See [`enforce_stat`]. +/// +/// Only the slots the daemon still reports. The kernel also counts the +/// legacy Fast Allow outcomes, and the slot layout stays as it is. #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] pub(super) struct EnforceStats { - /// `connect()` calls allowed because the map held an allow. - pub allowed: u64, /// `connect()` calls refused in-kernel, before a packet existed. pub denied: u64, /// `connect()` calls with no entry, which went on to the packet path. pub unknown: u64, - /// Grants the kernel saw but did not honour because the deadline had - /// lapsed - a daemon that stopped heartbeating, seen from the kernel. - pub stale: u64, - /// Grants not applied because the socket carried a foreign mark - a VPN - /// or proxy marking its own sockets, left alone on purpose. - pub foreign_mark: u64, /// Decisions the kernel made and could not report, because the ring was - /// full. Non-zero means the daemon's view of the fast path undercounts. + /// full. pub report_dropped: u64, } @@ -1464,6 +834,7 @@ pub(super) fn prepare() -> anyhow::Result { unpin_other_versions(&ns); let dir = pin_dir(); std::fs::create_dir_all(&dir).with_context(|| format!("creating {}", dir.display()))?; + remove_legacy_pins(&dir); Ok(dir) } @@ -1520,28 +891,85 @@ pub(super) fn already_attached(dir: &Path) -> bool { dir.join("connect4").exists() && dir.join("connect6").exists() } -/// A directory `attach` creates beside the connect pins once *both* cookie -/// connect variants have attached, and nothing else. +/// Removes what a 0.4-0.6 daemon pinned for Fast Allow and nothing reads any +/// more: the two sendmsg links and the cookie-variant marker directory. /// -/// The inherited path needs to know which connect variant a previous daemon -/// left running, and the pin names do not say: `connect4` is `connect4` -/// whether it holds the cookie program or the basic twin. The sendmsg pins used -/// to be the evidence - written only after the cookie variants took - which -/// made a daemon that ran cookie variants *without* sendmsg (a kernel that -/// refuses `bpf_getsockopt` there) look, to its successor, like a basic-connect -/// daemon, and refuse the fast path on every restart. bpffs allows directories, -/// so the evidence is now its own object and says one thing only. -pub(super) const COOKIE_MARKER: &str = "cookie-variants"; - -/// True when a previous daemon left the marker: the pinned connect programs -/// are the cookie variants, which carry `mark_decision`. -pub(super) fn cookie_variants_pinned(dir: &Path) -> bool { - dir.join(COOKIE_MARKER).is_dir() +/// Removing a link pin drops the kernel's last reference and detaches the +/// program, so this is also what takes an older daemon's inert sendmsg hooks +/// off the cgroup, on the inherited path as much as on a fresh attach. Absent +/// is the ordinary case and says nothing. The legacy *maps* stay pinned: the +/// connect programs still read them, and [`disarm_legacy_fast_allow`] needs +/// to reach the pinned copies. +fn remove_legacy_pins(dir: &Path) { + for name in [LINK_SENDMSG4, LINK_SENDMSG6] { + let pin = dir.join(name); + match std::fs::remove_file(&pin) { + Ok(()) => debug!("removed the legacy {name} pin at {}", pin.display()), + Err(e) if e.kind() == io::ErrorKind::NotFound => {} + Err(e) => warn!( + "could not remove the legacy {name} pin at {}: {e}", + pin.display() + ), + } + } + let marker = dir.join(LEGACY_COOKIE_MARKER); + match std::fs::remove_dir(&marker) { + Ok(()) => debug!("removed the legacy marker at {}", marker.display()), + Err(e) if e.kind() == io::ErrorKind::NotFound => {} + Err(e) => warn!( + "could not remove the legacy marker at {}: {e}", + marker.display() + ), + } } -/// True when a previous daemon left both sendmsg programs pinned. -pub(super) fn sendmsg_pinned(dir: &Path) -> bool { - dir.join(LINK_SENDMSG4).exists() && dir.join(LINK_SENDMSG6).exists() +/// Withdraws whatever Fast Allow state the pinned maps still hold: a zero +/// deadline, the unarmed mark, and no grants. +/// +/// The daemon no longer grants, but the kernel object still carries the maps +/// (ABI v4) and the connect programs still consult them. They are pinned, so +/// a 0.4-0.6 daemon that died while armed left a mark the hooks would go on +/// setting across any number of restarts - a mark that can collide with the +/// fwmark selectors of kube-proxy, Tailscale or wg-quick. With `UNARMED` +/// written, `mark_decision` returns at its first array read. Best effort: a +/// map that is missing or will not take the write is logged and skipped. +pub(super) fn disarm_legacy_fast_allow(bpf: &mut Ebpf) { + if let Some(m) = bpf.map_mut(MAP_FAST_ALLOW_UNTIL) { + match aya::maps::Array::<_, u64>::try_from(m) { + Ok(mut until) => { + if let Err(e) = until.set(0, 0u64, 0) { + warn!("could not zero the legacy {MAP_FAST_ALLOW_UNTIL}: {e}"); + } + } + Err(e) => warn!("{MAP_FAST_ALLOW_UNTIL} is not an array: {e}"), + } + } + if let Some(m) = bpf.map_mut(MAP_FAST_ALLOW_MARK) { + match aya::maps::Array::<_, u32>::try_from(m) { + Ok(mut mark) => { + if let Err(e) = mark.set(0, cfc_ebpf_common::fast_allow::UNARMED, 0) { + warn!("could not unarm the legacy {MAP_FAST_ALLOW_MARK}: {e}"); + } + } + Err(e) => warn!("{MAP_FAST_ALLOW_MARK} is not an array: {e}"), + } + } + if let Some(m) = bpf.map_mut(MAP_FAST_ALLOW) { + match BpfHashMap::<_, u32, u32>::try_from(m) { + Ok(mut grants) => { + let pids: Vec = grants.keys().flatten().collect(); + for pid in pids { + match grants.remove(&pid) { + Err(e) if !is_absent(&e) => { + warn!(pid, "could not drop a legacy {MAP_FAST_ALLOW} grant: {e}") + } + _ => {} + } + } + } + Err(e) => warn!("{MAP_FAST_ALLOW} is not a hash map: {e}"), + } + } } /// Unpins any lone connect-link leftovers so a fresh attach starts clean. @@ -1552,19 +980,7 @@ pub(super) fn sendmsg_pinned(dir: &Path) -> bool { /// the packet path like any other unenforced moment - is the price of not /// being wedged forever. fn drop_half_attached(dir: &Path) { - // The marker goes with the pins it describes; a marker outliving them - // would tell the next daemon the cookie variants are running when nothing - // is. - let marker = dir.join(COOKIE_MARKER); - if marker.is_dir() { - if let Err(e) = std::fs::remove_dir(&marker) { - warn!( - "could not remove the stale cookie marker at {}: {e}", - marker.display() - ); - } - } - for name in ["connect4", "connect6", LINK_SENDMSG4, LINK_SENDMSG6] { + for name in ["connect4", "connect6"] { let pin = dir.join(name); if !pin.exists() { continue; @@ -1639,56 +1055,17 @@ fn attach_one( Ok(insns) } -/// What [`attach`] managed to put in place. -pub(super) struct AttachedPrograms { - /// Every program attached, with its verified instruction count. - pub programs: Vec<(String, Option)>, - /// Whether the kernel side of the fast path is in place, and if not, the - /// one sentence that says why - two different kernels give two different - /// answers, and reporting the wrong one sent a reader to the wrong - /// kernel version. - pub fast_path: FastPathCapability, -} - -// Defined in `ebpf.rs`, where the `Report` that carries it lives in every -// build; re-exported so the paths this module's callers use keep resolving. -pub(super) use super::FastPathCapability; - -impl FastPathCapability { - /// The one reason the fast path cannot run at all, or `None`. - pub(super) fn refusal(self) -> Option<&'static str> { - match self { - Self::BasicConnect => Some( - "the connect hooks fell back to the basic variants, which carry no mark \ - decision (usually no bpf_get_socket_cookie / bpf_setsockopt on sock_addr \ - programs; the log line beside this one has the kernel's actual answer)", - ), - Self::SendmsgUnavailable => Some("fast-allow requires both UDP sendmsg hooks"), - Self::Ready => None, - } - } - - /// A guarantee the path runs without, for the report, or `None`. - pub(super) fn caveat(self) -> Option<&'static str> { - match self { - Self::SendmsgUnavailable => Some( - "the cgroup/sendmsg hooks did not load or attach, so a mark forged onto an \ - unconnected UDP socket is not stripped (usually this kernel's verifier: \ - bpf_getsockopt/setsockopt on sendmsg needs a newer kernel than on connect - \ - 5.10 refuses, 6.12 accepts; the log line beside this one has the actual answer)", - ), - Self::Ready | Self::BasicConnect => None, - } - } -} - /// Attaches both connect programs, pinning them under `dir` when it is -/// `Some`, then the sendmsg pair when the cookie variants took. +/// `Some`, and returns every program attached with its verified instruction +/// count. /// /// `dir` is `None` when [`prepare`] failed: the programs still attach and still /// enforce, they just stop when this process does. That is strictly better than /// not attaching, and worse than pinning, so the caller says which happened. -pub(super) fn attach(bpf: &mut Ebpf, dir: Option<&Path>) -> anyhow::Result { +pub(super) fn attach( + bpf: &mut Ebpf, + dir: Option<&Path>, +) -> anyhow::Result)>> { let root = super::cgroup::v2_root() .ok_or_else(|| anyhow!("no cgroup2 mount in /proc/mounts (unified hierarchy required)"))?; // Read-only, for the same reason as the DNS attach: the kernel wants the @@ -1705,7 +1082,6 @@ pub(super) fn attach(bpf: &mut Ebpf, dir: Option<&Path>) -> anyhow::Result = Vec::with_capacity(4); - let mut cookie_variants = 0usize; for (name, basic, pin_name) in [ (PROG_CONNECT4, PROG_CONNECT4_BASIC, "connect4"), (PROG_CONNECT6, PROG_CONNECT6_BASIC, "connect6"), @@ -1723,7 +1099,6 @@ pub(super) fn attach(bpf: &mut Ebpf, dir: Option<&Path>) -> anyhow::Result) -> anyhow::Result i, @@ -1753,63 +1128,7 @@ pub(super) fn attach(bpf: &mut Ebpf, dir: Option<&Path>) -> anyhow::Result out.push((name.to_string(), i)), - Err(e) => { - warn!( - "{name} could not load or attach ({e:#}); the fast path runs without the sendmsg hooks - a mark forged onto an unconnected UDP socket is not stripped on this kernel" - ); - fast_path = FastPathCapability::SendmsgUnavailable; - break; - } - } - } - if fast_path != FastPathCapability::Ready { - // Leave no lone sendmsg pin behind for the next start to trip on. - if let Some(d) = dir { - for p in [LINK_SENDMSG4, LINK_SENDMSG6] { - let _ = std::fs::remove_file(d.join(p)); - } - } - } - } - Ok(AttachedPrograms { - programs: out, - fast_path, - }) + Ok(out) } /// Removes entries for pids that no longer exist. @@ -1863,11 +1182,8 @@ pub(super) fn stats(map: &PerCpuArray<&MapData, u64>) -> anyhow::Result anyhow::Result<()> { Ok(()) } -/// Arms the kernel side of the fast path and returns the mark it will set. -/// -/// The nftables side is not here: the table is normally not loaded yet when -/// the daemon starts (`colony-firewall-nft.service` waits for the daemon), -/// so the set is written by the heartbeat task, which retries until it can - -/// see `nft_arm_state`. A mark in the map with no element in the set is a -/// wasted `setsockopt`, never a bypass: the ruleset accepts a value only while -/// it is in the set, and the caller flushed the set unconditionally before -/// this ran. -/// -/// The deadline does stay zero until the heartbeat's first beat - but because -/// `VerdictSink::arm` zeroes it, not by itself. `FAST_ALLOW_UNTIL` is a pinned -/// map, so after an unclean death it holds whatever the previous daemon last -/// wrote, up to a minute into the future; this comment asserted the zero for -/// two releases before anything wrote it. -/// -/// The mark is 32 random bits, never zero (`UNARMED`). Random per start -/// because since kernel 5.17 `SO_MARK` needs only `CAP_NET_RAW`, which docker -/// grants by default: a value anyone could read out of a package would be a -/// bypass token for any such process on the host's network. -fn arm_kernel_side(sink: &enforce::VerdictSink, configured: Option) -> anyhow::Result { - let mark = match configured { - Some(m) if m == cfc_ebpf_common::fast_allow::UNARMED => { - return Err(anyhow!( - "[ebpf] fast_allow_mark = 0 is not a mark: zero is what every socket \ - nothing has marked carries, and accepting it would accept everything" - )) - } - Some(m) => { - if let Some(who) = collides_with(m) { - // Their machine, their call - but not silently. - warn!( - "the configured fast-allow mark 0x{m:08x} is one {who} selects on; \ - traffic this daemon marks may be routed or dropped by that rule" - ); - } - m - } - None => pick_mark(|| uuid::Uuid::new_v4().as_u128() as u32).ok_or_else(|| { - anyhow!( - "could not draw a fast-allow mark that avoids the fwmark selectors this \ - host is likely to use; set [ebpf] fast_allow_mark to choose one by hand" - ) - })?, - }; - sink.arm(mark) - .context("writing the fast-allow mark to the kernel")?; - Ok(mark) -} - -/// fwmark selectors this machine is likely to already have, as -/// (mask, value, who) - a candidate `m` collides when `m & mask == value`. -/// -/// The mark is one 32-bit word shared by everything on the host, and the -/// dangerous consumers are the ones that select on a *mask*: they do not need -/// to guess our value, only to share a bit with it. A uniformly random word -/// therefore collides at a rate the other rule's mask decides, freshly at every -/// daemon start, which turns this into an intermittent and very hard to -/// attribute network fault - the fast path is off by default, so the operator's -/// first evidence is that turning it on breaks their VPN one boot in N. -/// -/// The two kube-proxy entries are why this list is not optional. Their masks -/// are a single bit, so a random word matches one of them **half the time**, -/// and `0x8000/0x8000` is the mark kube-proxy attaches to packets it then -/// DROPs. On such a node the previous code broke every fast-allowed flow on -/// roughly every other daemon start. -/// -/// This list is not, and cannot be, complete: nothing enumerates the fwmark -/// users of a Linux host. It is the documented ones, and `[ebpf] -/// fast_allow_mark` is the answer for a machine with a selector it misses. -const KNOWN_SELECTORS: &[(u32, u32, &str)] = &[ - // kube-proxy: masquerade, and drop. - (0x0000_4000, 0x0000_4000, "kube-proxy (masquerade)"), - (0x0000_8000, 0x0000_8000, "kube-proxy (drop)"), - // Tailscale's ip rules: "came from tailscale0", and "bypass tailscale". - (0x00ff_0000, 0x0008_0000, "Tailscale"), - (0x00ff_0000, 0x0004_0000, "Tailscale (bypass)"), - // wg-quick's `ip rule not fwmark lookup `: an exact-word - // compare, so this one costs a single value out of four billion. Listed - // because excluding it is free and the failure - the tunnel's own table - // stops being consulted for our traffic - is silent. - (0xffff_ffff, 0x0000_ca6c, "wg-quick"), -]; - -/// The first entry of [`KNOWN_SELECTORS`] that would match `mark`. -fn collides_with(mark: u32) -> Option<&'static str> { - KNOWN_SELECTORS - .iter() - .find(|(mask, value, _)| mark & mask == *value) - .map(|(_, _, who)| *who) -} - -/// Draws a mark that is neither `UNARMED` nor something in -/// [`KNOWN_SELECTORS`]. -/// -/// Rejection sampling rather than a claimed range, because a range is the -/// thing that must not be predictable: `SO_MARK` needs only CAP_NET_RAW since -/// 5.17, so a value an attacker can enumerate is a bypass token. Roughly a -/// quarter of the word survives the sieve - the two single-bit kube-proxy -/// masks account for almost all of it - which leaves about thirty bits of -/// entropy and takes four draws on average. -fn pick_mark(mut draw: impl FnMut() -> u32) -> Option { - // Bounded so a caller whose `draw` is degenerate cannot hang the daemon. - // About a quarter of the word survives the sieve, so 64 consecutive - // rejections is not chance - it is a broken source of randomness. - for _ in 0..64 { - let candidate = draw(); - if candidate != cfc_ebpf_common::fast_allow::UNARMED && collides_with(candidate).is_none() { - return Some(candidate); - } - } - // And then nothing, rather than a fallback. - // - // The obvious fallback - walk upward from 1 until the sieve passes - was - // worse than no fast path at all: it does not depend on `draw`, so it is - // the *same* value on every machine that reaches it. A published constant - // is precisely the bypass token the random draw exists to avoid, and - // `SO_MARK` needs only CAP_NET_RAW since 5.17. The path stays off, with - // the reason in `cfc status`, and every connection keeps taking the queue - - // which is the behaviour this whole feature degrades to anyway. - None -} - -/// The (deadline, heartbeat) pair a daemon should use, in seconds. -/// -/// With every guarantee in place the deadline is a backstop and the full pair -/// applies. With any reduced - see [`fast_path_decision`] - it is ten times -/// shorter and refreshed five times as often, and where exit detection is -/// imprecise that same beat is also the cadence of the stale-grant sweep. -fn deadline_pair(reduced: bool) -> (u64, u64) { - use cfc_ebpf_common::fast_allow as fa; - if reduced { - (fa::DEADLINE_SECS_REDUCED, fa::HEARTBEAT_SECS_REDUCED) - } else { - (fa::DEADLINE_SECS, fa::HEARTBEAT_SECS) - } -} - -/// What the eligibility decision is made from, so the decision can be a pure -/// function with a test rather than a ladder of `else if` that only a live -/// kernel exercises. -#[derive(Debug, Clone, Copy)] -struct LadderFacts { - config_on: bool, - has_maps: bool, - exit_tracking: bool, - exit_precise: bool, - lifecycle_pinned: bool, - capability: enforce::FastPathCapability, -} - -/// Legacy status rendering remains readable for older clients. -#[cfg(test)] -const REDUCED_IMPRECISE_EXIT: &str = "exit is detected by thread-group leader only"; - -/// Legacy capability checks remain conservative, but runtime grants are disabled -/// for every configuration: a socket mark does not identify its current sender. -const FAST_ALLOW_DISABLED: &str = - "Fast Allow is disabled: socket marks cannot verify the current sender; use normal NFQUEUE filtering"; - -fn fast_path_decision(f: &LadderFacts) -> Result, &'static str> { - if !f.config_on { - return Err("[ebpf] fast_allow is not set"); - } - if !f.has_maps { - return Err("the loaded object has no fast-allow maps"); - } - if !f.exit_tracking || !f.exit_precise || !f.lifecycle_pinned { - return Err("fast-allow requires precise, pinned exec/exit hooks"); - } - if let Some(why) = f.capability.refusal() { - return Err(why); - } - Err(FAST_ALLOW_DISABLED) -} - -/// One attempt at the nftables side, at startup, reported as the state it -/// leaves the path in. On a daemon *restart* the table is already loaded and -/// this comes back `Live` at once; on a boot it comes back waiting, and the -/// heartbeat task finishes the job. -fn nft_arm_state(mark: u32, deadline_secs: u64, reduced: Option) -> FastAllow { - match super::nft_set::arm(mark) { - Ok(()) => FastAllow::Live { - deadline_secs, - reduced, - }, - Err(e) => nft_arm_state_from_error(&e), - } -} - -/// The reported state for a failed nftables arm. A missing *table* is the -/// ruleset unavailable and reads as waiting; a missing *set* is an operator-visible fact - a snippet that -/// predates the feature - and carries the fix; anything else is quoted. -fn nft_arm_state_from_error(e: &anyhow::Error) -> FastAllow { - match e.downcast_ref::() { - Some(super::nft_set::Absent::Table) => FastAllow::Off( - "waiting for the nftables table (colony-firewall-nft.service has not installed filtering)" - .to_string(), - ), - Some(super::nft_set::Absent::Set) => FastAllow::Off(format!("{e}")), - None => FastAllow::Off(format!("could not arm nftables: {e:#}")), - } -} - /// Classifies a failure from `EbpfLoader::load` - parsing the ELF, creating /// maps, applying relocations. /// @@ -401,18 +196,6 @@ impl Drop for Attached { for t in &self.tasks { t.abort(); } - // A clean stop disarms now rather than letting the deadline lapse: - // zero deadline and unarmed mark in the kernel, the set flushed in - // nftables. Best effort - a daemon on its way out has only the log. - if let Some(sink) = &self._sink { - sink.disarm(); - if let Err(e) = super::nft_set::disarm_for_shutdown() { - tracing::warn!( - "could not flush the fast-allow mark from nftables on shutdown: {e:#}" - ); - } - super::set_fast_allow_level(FastAllow::Off("the daemon stopped".to_string())); - } } } @@ -422,32 +205,12 @@ impl Drop for Attached { /// missing file, a malformed ELF, a kernel that refuses the whole program set. /// Individual attach failures are recorded in the [`Report`] and leave the /// rest running. -// Eight injected dependencies, each a different thing the layer may read or -// feed and none of which it should own; a bag struct to satisfy the lint -/// The `[ebpf]` fast-path settings one load should honour. -/// -/// Two fields rather than two parameters: the argument list is already at the -/// lint's limit, and these two are one decision - whether the fast path runs, -/// and with which mark - taken from one config section. -#[derive(Clone, Copy, Debug, Default)] -pub(super) struct FastAllowCfg { - /// `[ebpf] fast_allow`. - pub on: bool, - /// `[ebpf] fast_allow_mark`, when the operator pinned one. `None` draws. - pub mark: Option, -} - -// would name nothing that the parameter list does not already name. -#[allow(clippy::too_many_arguments)] pub(super) fn load_and_attach( object_path: &Path, dns: DnsCache, table: KernelProcTable, engine: Option, trust: Trust, - observed: tokio::sync::broadcast::Sender, - stats: crate::stats::Stats, - fast_allow: FastAllowCfg, ) -> Result<(Attached, Report), LoadError> { let mut report = Report { mode: crate::config::EbpfMode::On, @@ -455,36 +218,6 @@ pub(super) fn load_and_attach( ..Report::default() }; - // Whatever a previous daemon left accepted in the nftables set, drop it - - // first, before anything can return. - // - // This lived further down for a while, next to the code that arms, and - // that was wrong twice over. It ran only when this daemon was *eligible* - // and armed; and even after being made unconditional it still sat behind - // every early return in this function - a missing object (which is the - // single most common outcome on a default install), an untrusted one, a - // failed load. So a daemon that crashed while armed and came back to any - // of those left its predecessor's mark sitting in the set: accepted by the - // ruleset, refreshed by nobody, removed by nothing short of the table - // going away. That is a standing bypass token rather than a stale entry - - // every process that was ever fast-allowed can read the value back off its - // own socket with `getsockopt(SO_MARK)`, and setting it again needs only - // CAP_NET_RAW. - // - // Flushing before knowing whether this daemon will arm is the right order: - // an empty set accepts nothing, which is the safe state to pass through. - if let Err(e) = super::nft_set::disarm_for_start() { - // Logged as well as noted, because the note alone reaches nobody on - // the paths that matter most: every early return below builds a - // `LoadError` and drops this `Report` on the floor, and a failed flush - // followed by a failed load is exactly the shape that leaves a - // predecessor's mark accepted with no daemon to explain it. - tracing::error!("could not disable previous Fast Allow state: {e:#}; old marks may still bypass filtering; run systemctl reload colony-firewall-nft and inspect the journal before relying on filtering"); - report.notes.push(format!( - "could not disable previous Fast Allow state: {e:#}; old marks may still bypass filtering; reload colony-firewall-nft before relying on filtering" - )); - } - // Vet before read, so a file we would refuse is never even pulled into // memory, and so the "not there at all" case is distinguishable from the // "there but not ours" one. @@ -595,11 +328,10 @@ pub(super) fn load_and_attach( // Attribution rather than enforcement, but the same restart // split applies; see the SOCK_PIDS paragraph below. (enforce::MAP_SOCK_PIDS, dir.join(enforce::MAP_SOCK_PIDS)), - // The fast path's four. Pinned for the same restart reason - // as everything above, and it is the pinning that makes the - // deadline load-bearing: the programs keep these alive after - // the daemon dies, so only `FAST_ALLOW_UNTIL` running out - // stops the marks. + // The legacy Fast Allow maps. Nothing grants any more, but + // the kernel object still reads them, and they must be the + // pinned ones or the startup disarm writes a fresh map that + // no inherited program sees. (enforce::MAP_FAST_ALLOW, dir.join(enforce::MAP_FAST_ALLOW)), ( enforce::MAP_FAST_ALLOW_UNTIL, @@ -859,6 +591,12 @@ pub(super) fn load_and_attach( }) .map_err(|e| LoadError::new(classify_load(&e), e))?; + // Fast Allow is gone from the daemon, but the kernel object still carries + // its maps (ABI v4) and they are pinned, so a 0.4-0.6 daemon that died + // while armed left a mark the connect hooks would go on setting, past any + // restart. Disarm before anything attaches, whatever else comes up. + enforce::disarm_legacy_fast_allow(&mut bpf); + // --- attach, each independently ------------------------------------ let exec_pin = pin_dir.as_deref().map(|d| d.join(enforce::LINK_EXEC)); @@ -893,35 +631,15 @@ pub(super) fn load_and_attach( &mut exit_pinned, ); report.exit_tracking = record_attach(&mut report, PROG_EXIT, "sched_process_exit", r); - // Both clears have to survive this daemon for a grant to be safe past its - // death, so this is one flag, not two. + // Both clears have to survive this daemon for its denials to stay + // honest past its death, so this is one flag, not two. report.lifecycle_pinned = exec_pinned && exit_pinned; let r = attach_dns(&mut bpf); report.dns_capture = record_attach(&mut report, PROG_DNS, "cgroup_skb/ingress", r); // --- enforcement ---------------------------------------------------- - // Whether the kernel side of the fast path is in place. On the inherited - // path the pin names do not say which connect variant is running; the - // previous daemon's cookie marker does (see `enforce::COOKIE_MARKER`), and - // the sendmsg pins say whether that daemon had those hooks too. - let mut fast_path = enforce::FastPathCapability::BasicConnect; report.enforcement = if inherited { - fast_path = match pin_dir.as_deref() { - Some(d) if enforce::cookie_variants_pinned(d) => { - if enforce::sendmsg_pinned(d) { - enforce::FastPathCapability::Ready - } else { - enforce::FastPathCapability::SendmsgUnavailable - } - } - // No marker: a basic-connect daemon, or one that could not create - // the marker and said so. Either way nothing here is known to mark, - // so the packet path decides - fail closed, and the reason names - // the marker so the fix (a restart with the pins removed) is - // legible. - _ => enforce::FastPathCapability::BasicConnect, - }; report.notes.push(format!( "in-kernel enforcement was already attached and pinned at {}; \ steering it rather than replacing it", @@ -930,9 +648,8 @@ pub(super) fn load_and_attach( Enforcement::Inherited } else { match enforce::attach(&mut bpf, pin_dir.as_deref()) { - Ok(attached) => { - fast_path = attached.fast_path; - for (name, insns) in attached.programs { + Ok(programs) => { + for (name, insns) in programs { if let Some(n) = insns { report.verified_insns.push((name, n)); } @@ -953,13 +670,6 @@ pub(super) fn load_and_attach( } }; - // A fact of the eligibility ladder, recorded whether or not the ladder - // runs below: a layer handed no engine still reports what the kernel let - // it have, which is what the kernel matrix reads. `None` where nothing - // attached - `fast_path` still says BasicConnect then, and that would be - // a claim about hooks that do not exist. - report.fast_path_capability = report.enforcement.is_live().then_some(fast_path); - // The socket-cookie -> pid map, for O(1) attribution - the pinned one // when there is a pin directory, which is what lets an inherited connect // program's writes land somewhere this daemon can read. Taken whenever it @@ -1009,26 +719,15 @@ pub(super) fn load_and_attach( .and_then(|m| aya::maps::PerCpuArray::<_, u64>::try_from(m).map_err(Into::into)) .and_then(|m| enforce::stats(&m)) { - // Every counter, in the guard and in the message. Two of them - // used to be read and then left out of both, so the one state an - // operator most wants named - a foreign mark keeping the fast path - // permanently disengaged for some program - could not be reached - // from the note at all. - Ok(s) - if s.denied > 0 - || s.allowed > 0 - || s.unknown > 0 - || s.stale > 0 - || s.foreign_mark > 0 - || s.report_dropped > 0 => - { + // The counters the daemon still acts on. The kernel keeps + // counting the legacy Fast Allow slots too, and the stat layout + // stays as it is, but those are only ever zero now. + Ok(s) if s.denied > 0 || s.unknown > 0 || s.report_dropped > 0 => { report.notes.push(format!( "in-kernel enforcement carried over: {} connect() refused, \ - {} fast-allowed, {} passed to the packet path, {} grants \ - ignored as stale, {} sockets left alone for carrying \ - another marker's mark, {} decisions the report ring could \ - not hold, since the pins were made", - s.denied, s.allowed, s.unknown, s.stale, s.foreign_mark, s.report_dropped + {} passed to the packet path, {} decisions the report ring \ + could not hold, since the pins were made", + s.denied, s.unknown, s.report_dropped )) } Ok(_) => {} @@ -1066,121 +765,19 @@ pub(super) fn load_and_attach( // enforcement did not come up, or when the caller has no rule engine to // consult (the live tests); in both cases the map simply stays empty and // every connect falls through to the packet path. - // The mark the kernel side was armed with, when it was: the heartbeat - // task below needs it to finish the nftables half of arming. Alongside it, - // the deadline pair that task must write and pace itself by - the full one, - // or the shortened one for a kernel whose lifecycle links could not be - // pinned. Both are decided inside the block below and used after it. - let mut armed_mark: Option = None; - let mut deadline_secs = cfc_ebpf_common::fast_allow::DEADLINE_SECS; - let mut heartbeat_secs = cfc_ebpf_common::fast_allow::HEARTBEAT_SECS; - // And the reason the guarantee is weaker, if it is, for every `Live` the - // heartbeat will ever publish. Hoisted rather than read back out of - // `report.fast_allow` at spawn time, because on a boot that field is - // `Off("waiting for the nftables table")` when the task starts - the - // common case, not an edge - and deriving from it lost the reason on - // every first arm. - let mut reduced_because: Option = None; let sink = match (report.enforcement.is_live(), engine) { (true, Some(engine)) => { match enforce::VerdictSink::new(&mut bpf, engine.clone(), table.clone()) { - Ok(mut sink) => { - // Whatever the previous daemon granted, it granted under - // its rules. The map is pinned, so those grants are still - // here; nothing below writes a grant until this is done. - let dropped = sink.flush_fast_allow(); - if dropped > 0 { - debug!( - "dropped {dropped} fast-allow grants inherited from a previous daemon" - ); - } - - // The decision is a pure function of the facts, so it has a - // test; this is only the gathering. `enforcement` is not - // among the facts - see `fast_path_decision` for why the - // Process-mode refusal was backwards. - let facts = LadderFacts { - config_on: fast_allow.on, - has_maps: sink.has_fast_path(), - exit_tracking: report.exit_tracking, - exit_precise: report.exit_precise, - lifecycle_pinned: report.lifecycle_pinned, - capability: fast_path, - }; - let (off, reduced): (Option<&str>, Vec<&str>) = match fast_path_decision(&facts) - { - Ok(reduced) => (None, reduced), - Err(why) => (Some(why), Vec::new()), - }; - - // Weaker guarantees are said, once each, in the log and the - // report, and carried in the status so `live` never hides - // them. Both select the short deadline pair; where exit - // detection is imprecise the heartbeat also sweeps grants, - // which is what makes that reduction boundable at all. - (deadline_secs, heartbeat_secs) = deadline_pair(!reduced.is_empty()); - reduced_because = if off.is_none() && !reduced.is_empty() { - for why in &reduced { - let note = format!( - "fast-allow runs with a weaker guarantee: {why}; grants lapse within {deadline_secs}s instead of {}s", - cfc_ebpf_common::fast_allow::DEADLINE_SECS - ); - warn!("{note}"); - report.notes.push(note); - } - Some(reduced.join("; ")) - } else { - None - }; - if off.is_none() { - if let Some(caveat) = fast_path.caveat() { - warn!("fast-allow: {caveat}"); - report.notes.push(format!("fast-allow: {caveat}")); - } - } - - let (state, mark_opt) = match off { - Some(why) => { - if fast_allow.on { - warn!("{FAST_ALLOW_DISABLED}"); - report.notes.push(FAST_ALLOW_DISABLED.to_string()); - } - sink.withdraw_fast_path(); - (FastAllow::Off(why.to_string()), None) - } - None => match arm_kernel_side(&sink, fast_allow.mark) { - Ok(mark) => ( - nft_arm_state(mark, deadline_secs, reduced_because.clone()), - Some(mark), - ), - Err(e) => { - sink.withdraw_fast_path(); - (FastAllow::Off(format!("could not arm: {e:#}")), None) - } - }, - }; - report.fast_allow = Some(state); - armed_mark = mark_opt; - + Ok(sink) => { let sink = std::sync::Arc::new(sink); // resync rather than compile_rules alone: on the inherited // path the pinned map holds the previous daemon's // verdicts, made under the previous daemon's rules, and // this is the reconciliation that makes them this - // daemon's. - // - // Which of its parts does that work is worth being exact - // about, because a comment here once claimed the orphan - // sweep did all of it and that was only half true. The - // proc table is empty at this point - `set_live` has not - // run yet - so the live loop no-ops, and the orphan sweep - // reconciles the *denials* the previous daemon left in - // `VERDICTS`. It cannot reconcile grants: `flush_fast_allow` - // above has just emptied the map the sweep would walk, on - // purpose. Re-seeding the grants is `sweep_fast_allow`'s - // job, at the end of `resync`, and it walks /proc rather - // than any map - which is the only way to reach a process - // that was already running when this daemon started. + // daemon's. The proc table is empty at this point - + // `set_live` has not run yet - so the live loop no-ops, + // and it is the orphan sweep that reconciles the denials + // the previous daemon left in `VERDICTS`. sink.resync(); let weak = std::sync::Arc::downgrade(&sink); engine.set_on_change(Box::new(move || { @@ -1200,32 +797,6 @@ pub(super) fn load_and_attach( } _ => None, }; - if report.fast_allow.is_none() { - report.fast_allow = Some(FastAllow::Off( - if report.enforcement.is_live() { - "no decision engine was handed to the layer" - } else { - "in-kernel enforcement is not live" - } - .to_string(), - )); - } - - // Published here, and only here, because after this point the heartbeat - // task below is running and publishing states of its own. - // - // `start` used to do it, once `load_and_attach` returned - which is *after* - // that task exists. On a daemon restart the table is already loaded, so the - // very first thing the heartbeat does is arm and publish `Live`; `start` - // then overwrote it with the state decided here, "waiting for the nftables - // table". And nothing ever corrected it: the heartbeat only publishes while - // it is not armed. `cfc status` said the fast path was waiting for a table - // that had been there all along, for the life of the daemon, while the path - // was in fact live. - if let Some(state) = report.fast_allow.clone() { - super::set_fast_allow_level(state); - } - // Exec without exit tracking would let entries age out on the TTL alone, // which is a materially weaker pid-reuse story. Refuse the combination // rather than quietly serving it. @@ -1303,35 +874,6 @@ pub(super) fn load_and_attach( } } - // The eligibility ladder ran before any of the consumers above existed, - // and two of them can retract what it assumed: a ring consumer that fails - // to start turns `exec_tracking` off, and the exit one turns both off. So - // the fast path could be armed, reported `live`, and marking sockets while - // `on_exec` - its only per-execve writer - could never run, and while - // nothing on the daemon side evicted a grant. - // - // Correct it here rather than moving the decision, because the decision - // needs the sink and the sink is what these consumers borrow. Flush what - // was granted, empty the set so the ruleset accepts nothing, and leave - // `armed_mark` unset so the heartbeat task below is never spawned - with - // no heartbeat the kernel stops honouring grants within one deadline, and - // with no element in the set it stops mattering immediately. - if armed_mark.is_some() && !(report.exec_tracking && report.exit_tracking) { - let why = "the exec/exit ring consumers did not start, so nothing would grant or evict"; - warn!("fast-allow withdrawn after arming: {why}"); - if let Err(e) = super::nft_set::disarm() { - report - .notes - .push(format!("could not flush the fast-allow set: {e:#}")); - } - if let Some(sink) = sink.as_ref() { - sink.flush_fast_allow(); - } - armed_mark = None; - report.fast_allow = Some(FastAllow::Off(why.to_string())); - super::set_fast_allow_level(FastAllow::Off(why.to_string())); - } - // Denials refused in the kernel never reach NFQUEUE, so this consumer is // the only thing standing between "CFC blocked it" and "the connection just // failed". It is a log line rather than a prompt on purpose: the user @@ -1363,219 +905,6 @@ pub(super) fn load_and_attach( } } - // The fast path's two tasks, only while it is live: the heartbeat that - // keeps the kernel honouring grants, and the consumer that keeps the - // rest of the daemon honest about flows the packet path never sees. - if let (Some(mark), Some(sink)) = (armed_mark, sink.as_ref()) { - // Heartbeat, and the nftables side of arming. The two are one task on - // purpose: `colony-firewall-nft.service` is After= this daemon and - // waits for it to be active, so at daemon start the table is not - // loaded yet and the set cannot be written - on every boot, not as - // an edge case. The kernel side is armed (mark written) but the - // deadline stays zero, so nothing is honoured, until the element is - // in the set; then every tick refreshes the deadline. If this task - // ever stops - abort on shutdown, a wedged runtime, the daemon dying - // - the kernel stops honouring grants within one deadline. That is - // the design, not a failure mode. - let beat = sink.clone(); - let mut armed = matches!(report.fast_allow, Some(FastAllow::Live { .. })); - // What the status must keep saying every time this task re-arms, and - // whether each beat also sweeps grants. - let reduced_for_status = reduced_because.clone(); - let sweep_grants = !report.exit_precise; - tasks.push(tokio::spawn(async move { - let mut tick = tokio::time::interval(std::time::Duration::from_secs(heartbeat_secs)); - let mut last_reason: Option = None; - // Ticks between two checks that the set still holds the mark. - // Expressed in ticks, so it has to follow the tick length: one - // minute either way, whichever deadline pair is in force. - // Clamped at both ends: a divisor of zero would panic, and a - // heartbeat longer than the check period would make this zero, - // which reads as "check on every tick" - a fork and exec every - // beat, forever. - let checks_every: u32 = ((60 / heartbeat_secs.max(1)) as u32).max(1); - let mut since_check: u32 = 0; - loop { - tick.tick().await; - if !armed { - // Off the async threads: this execs nft and waits on it. - let attempt = tokio::task::spawn_blocking(move || super::nft_set::arm(mark)) - .await - .unwrap_or_else(|e| Err(anyhow!("arming task failed: {e}"))); - let state = match attempt { - Ok(()) => { - armed = true; - tracing::info!( - "fast-allow armed: the nftables set now holds this daemon's mark" - ); - FastAllow::Live { - deadline_secs, - reduced: reduced_for_status.clone(), - } - } - Err(e) => nft_arm_state_from_error(&e), - }; - // Say each reason once, not on every beat. - let reason = state.describe(); - if last_reason.as_deref() != Some(reason.as_str()) { - if !armed { - tracing::info!("fast-allow {reason}"); - } - last_reason = Some(reason); - } - super::set_fast_allow_level(state); - if !armed { - continue; - } - } - if let Err(e) = beat.beat(deadline_secs) { - // The number, not "a minute": on a kernel whose lifecycle - // links could not be pinned this deadline is six seconds, - // and a warning that names the wrong one sends a reader - // looking for a window that closed long ago. - tracing::warn!( - "fast-allow heartbeat failed: {e:#}; grants lapse within {deadline_secs}s" - ); - } - - // On a kernel without `group_dead` the exit program evicts on - // leader exit only, so a process whose leader exits first and - // dies later keeps its grant while this daemon lives and - // refreshes the deadline. This is what bounds that: every - // beat, every granted pid is re-dated against the start time - // it was granted with. Off the async threads - it reads /proc - // once per granted pid, and granted pids are the few a lasting - // rule allows outright. - if sweep_grants { - let sweeper = beat.clone(); - match tokio::task::spawn_blocking(move || sweeper.sweep_stale_grants()).await { - Ok(0) => {} - Ok(n) => tracing::debug!(dropped = n, "swept stale fast-allow grants"), - // A panic inside the sweep; the grants it did not reach - // are re-checked next beat, so this is worth a line and - // nothing more. - Err(e) => tracing::debug!("fast-allow grant sweep did not run: {e}"), - } - } - - // Armed is not a fact that stays true, and this loop used to - // treat it as one: once the element went in, the only thing it - // ever did again was refresh the deadline. `systemctl restart - // nftables`, or any `nft -f` that reloads the machine's - // ruleset, recreates `table inet colony_firewall` with an - // empty set - and the daemon went on marking sockets, went on - // crediting rule hits from ALLOW_EVENTS, and went on telling - // `cfc status` that the fast path was live, while every one of - // those flows was in fact taking the queue and being counted - // a second time by the packet path. - // - // Checked once a minute rather than every tick: this is a - // fork and exec, the window it leaves is a minute of an - // over-optimistic status line, and nothing unsafe happens in - // it - the failure is the set accepting *less* than the daemon - // thinks, never more. A minute either way, so a shortened - // heartbeat does not turn this into a fork every two seconds. - since_check += 1; - if since_check >= checks_every { - since_check = 0; - match tokio::task::spawn_blocking(move || super::nft_set::holds(mark)).await { - Ok(Ok(true)) => {} - Ok(Ok(false)) => { - armed = false; - last_reason = None; - tracing::warn!( - "the fast-allow mark is no longer in the nftables set (the \ - ruleset was reloaded); re-arming" - ); - super::set_fast_allow_level(FastAllow::Off( - "the nftables set no longer holds this daemon's mark; re-arming" - .to_string(), - )); - } - // Could not ask. Say nothing and keep the current - // state: a failed probe is not evidence either way, - // and disarming on it would take the path down on a - // transient. - Ok(Err(e)) => tracing::debug!("could not check the fast-allow set: {e:#}"), - Err(e) => tracing::debug!("fast-allow set check did not run: {e}"), - } - } - } - })); - - // ALLOW_EVENTS: one record per flow the kernel waved past the queue. - // Credited to the rule that granted, counted where NFQUEUE counts, - // and fed to the same observed stream - so the busiest allow rule - // does not read as dead and the enforcing heuristic does not flip - // to "not enforcing" while the firewall is doing its job. - let credit = sink.clone(); - let engine_hits = sink.engine().clone(); - let observed_tx = observed.clone(); - let stats_tx = stats.clone(); - // The same reverse-DNS seam the packet path uses. Without it a - // fast-allowed flow is the one kind of connection whose destination - // never gets a name: the packet path attaches whatever is cached and - // enqueues a lookup for next time, and this consumer - which exists - // precisely because these flows never reach that path - did neither. - // So `cfc log` and the live feed showed bare addresses for exactly the - // programs a user had trusted enough to allow outright, and the cache - // was never warmed for their destinations either, so the *next* flow - // to the same host had no name to attach. - let dns_hosts = dns.clone(); - match spawn_ring(&mut bpf, enforce::MAP_ALLOW_EVENTS, move |bytes| { - let Some(ev) = decode::(bytes) else { - return; - }; - stats_tx.record_allow(); - let verdict = match credit.granted_by(ev.pid) { - Some(rule) => { - engine_hits.record_hit(rule); - cfc_core::Verdict::from_rule(cfc_core::Action::Allow, rule) - } - // A grant this daemon did not make (a previous one's, in the - // window before the startup flush). Reported, credited to no - // rule rather than to the wrong one. - None => cfc_core::Verdict::from_policy(cfc_core::Action::Allow), - }; - let protocol = match ev.protocol { - 6 => cfc_core::Protocol::Tcp, - 17 => cfc_core::Protocol::Udp, - other => cfc_core::Protocol::Other(other), - }; - let unspecified = if ev.family == 4 { - std::net::IpAddr::V4(std::net::Ipv4Addr::UNSPECIFIED) - } else { - std::net::IpAddr::V6(std::net::Ipv6Addr::UNSPECIFIED) - }; - let dst = ev.destination(); - let mut connection = cfc_core::Connection::new( - protocol, - cfc_core::Direction::Outbound, - unspecified, - 0, - dst.ip(), - dst.port(), - ); - if let Some((host, verified)) = dns_hosts.cached_named(dst.ip()) { - connection = connection.with_host_verified(host, verified); - } - dns_hosts.enqueue_lookup(dst.ip()); - let process = crate::process_resolve::resolve(ev.pid); - let _ = observed_tx.send(crate::nfqueue::ObservedConnection { - connection, - process, - verdict, - }); - }) { - Ok(task) => tasks.push(task), - Err(e) => report.notes.push(format!( - "{} consumer not started: {e:#}; fast-allowed flows will be \ - unreported and their rules uncredited", - enforce::MAP_ALLOW_EVENTS - )), - } - } - if report.dns_capture { let cache = dns.clone(); // One scratch answer for the life of the consumer. `for_each_answer` @@ -1709,14 +1038,10 @@ fn attach_tracepoint( // evicting after the daemon dies" property, which is what best-effort was // meant to mean. // - // `pinned_out` carries that outcome to the caller, because one feature does - // depend on it. The fast path's eligibility ladder asks for `Pinned` - // enforcement and exit tracking, on the reasoning that a grant is always - // cleared even if the daemon dies - and that reasoning is the *pin's*, not - // the attach's. On a kernel with no BPF_LINK_TYPE_PERF_EVENT the connect - // programs stay pinned and go on marking sockets while the exec and exit - // clears die with the daemon, which the ladder could not see because this - // function used to return the same `Ok` either way. + // `pinned_out` carries that outcome to the caller, which reports it as + // `lifecycle_pinned`: on a kernel with no BPF_LINK_TYPE_PERF_EVENT the + // connect programs stay pinned while the exec and exit clears die with + // the daemon, and this function used to return the same `Ok` either way. let pinned = prog .take_link(id) .map_err(anyhow::Error::new) @@ -1949,18 +1274,6 @@ mod tests { assert!(process_group_is_gone(u32::MAX)); } - #[test] - fn fast_allow_is_refused_even_with_every_hook_available() { - let facts = LadderFacts { - config_on: true, - has_maps: true, - exit_tracking: true, - exit_precise: true, - lifecycle_pinned: true, - capability: enforce::FastPathCapability::Ready, - }; - assert!(fast_path_decision(&facts).is_err()); - } use std::net::Ipv4Addr; use std::os::unix::fs::PermissionsExt as _; @@ -2025,150 +1338,6 @@ mod tests { assert!(!dir_is_safe(0, 0o040775), "group-writable counts too"); } - /// The regression this sieve exists for: kube-proxy selects on - /// `0x8000/0x8000` and DROPs what matches. A uniformly random word has - /// that bit set half the time, so on a Kubernetes node the previous - /// draw broke every fast-allowed flow on roughly every other start. - /// The property that makes a late tick harmless: several beats fit inside - /// one deadline, for *both* pairs. A ratio of one would mean a single - /// delayed heartbeat lapses the fast path on a perfectly healthy daemon. - #[test] - fn several_beats_fit_inside_every_deadline() { - for reduced in [false, true] { - let (deadline, heartbeat) = deadline_pair(reduced); - assert!(heartbeat > 0, "a zero heartbeat would spin"); - assert!( - deadline >= heartbeat * 3, - "reduced={reduced}: {deadline}s deadline against a {heartbeat}s beat leaves \ - no room for a late tick" - ); - } - } - - /// The unpinned pair exists to shrink the window a dead daemon leaves, so - /// it has to actually be shorter - and the pinned one has to be the value - /// every document quotes. - #[test] - fn the_reduced_deadline_is_the_shorter_one() { - let (full, _) = deadline_pair(false); - let (reduced, beat) = deadline_pair(true); - assert_eq!(full, cfc_ebpf_common::fast_allow::DEADLINE_SECS); - assert!( - reduced < full, - "a reduced guarantee must not get the longer deadline: {reduced} vs {full}" - ); - // The heartbeat must speed up with it, or the ratio above breaks. - assert!(beat < cfc_ebpf_common::fast_allow::HEARTBEAT_SECS); - } - - /// The policy, over the cases that decide it. Refusals are where nothing - /// could mark or nothing could evict; everything weaker but boundable - /// reduces; a missing sendmsg hook is a note. `Enforcement` is not an - /// input at all, which is itself the assertion. - #[test] - fn the_ladder_never_arms_socket_mark_authorization() { - use enforce::FastPathCapability as Cap; - for config_on in [false, true] { - for capability in [Cap::Ready, Cap::SendmsgUnavailable, Cap::BasicConnect] { - for lifecycle_pinned in [false, true] { - for exit_precise in [false, true] { - assert!(fast_path_decision(&LadderFacts { - config_on, - has_maps: true, - exit_tracking: true, - exit_precise, - lifecycle_pinned, - capability, - }) - .is_err()); - } - } - } - } - } - - /// `cfc status` must not say plain `live` on a kernel where the guarantee - /// is weaker - that is the whole reason the deadline is carried. - #[test] - fn a_shortened_deadline_is_visible_in_the_status_line() { - let full = FastAllow::Live { - deadline_secs: cfc_ebpf_common::fast_allow::DEADLINE_SECS, - reduced: None, - }; - assert_eq!(full.describe(), "live"); - - let short = FastAllow::Live { - deadline_secs: cfc_ebpf_common::fast_allow::DEADLINE_SECS_REDUCED, - reduced: Some(REDUCED_IMPRECISE_EXIT.to_string()), - }; - let said = short.describe(); - assert_ne!( - said, "live", - "a weaker guarantee must not read as the full one" - ); - assert!( - said.contains(&cfc_ebpf_common::fast_allow::DEADLINE_SECS_REDUCED.to_string()), - "the status line must name the number: {said}" - ); - assert!( - said.contains("leader only"), - "two different weaknesses give the same six seconds, so the status must say \ - which: {said}" - ); - } - - #[test] - fn a_mark_sharing_a_bit_with_a_known_selector_is_refused() { - assert_eq!(collides_with(0x0000_8000), Some("kube-proxy (drop)")); - assert_eq!(collides_with(0xdead_8000), Some("kube-proxy (drop)")); - assert_eq!(collides_with(0x0000_4000), Some("kube-proxy (masquerade)")); - assert_eq!(collides_with(0x0008_0000), Some("Tailscale")); - assert_eq!(collides_with(0x1208_0000), Some("Tailscale")); - assert_eq!(collides_with(0x0004_0000), Some("Tailscale (bypass)")); - // wg-quick's value is caught, though by kube-proxy's masquerade bit - // rather than by its own entry: 0xca6c has bit 14 set. Its entry is - // kept anyway - it documents the selector, and it is what would catch - // the value if the kube-proxy masks ever moved. - assert!(collides_with(0x0000_ca6c).is_some()); - - // And values no selector here claims. - assert_eq!(collides_with(0x0000_0a6c), None); - assert_eq!(collides_with(0x0003_3331), None); - } - - #[test] - fn a_drawn_mark_is_never_unarmed_and_never_collides() { - // A deterministic walk over the space rather than a real rng: the - // property is about the sieve, and a test that draws randomly would - // pass or fail randomly. - let mut seed = 0x1234_5678u32; - let mut draw = || { - seed = seed.wrapping_mul(1_664_525).wrapping_add(1_013_904_223); - seed - }; - for _ in 0..2000 { - let mark = pick_mark(&mut draw).expect("a healthy source always yields one"); - assert_ne!(mark, cfc_ebpf_common::fast_allow::UNARMED); - assert_eq!( - collides_with(mark), - None, - "drew a colliding mark 0x{mark:08x}" - ); - } - } - - /// A source that only ever offers unusable values must not hang the - /// daemon - and must not be answered with a *constant* either, which is - /// what an earlier fallback did: a value that does not depend on the draw - /// is the same on every machine that reaches it, which is the published - /// bypass token the random draw exists to avoid. Refusing leaves the fast - /// path off, which is where it degrades to anyway. - #[test] - fn a_degenerate_draw_arms_nothing_rather_than_a_constant() { - assert_eq!(pick_mark(|| 0x0000_8000), None); - assert_eq!(pick_mark(|| cfc_ebpf_common::fast_allow::UNARMED), None); - } - #[test] fn a_world_writable_object_is_refused_but_only_under_refuse() { let dir = tempfile::tempdir().expect("tempdir"); @@ -2184,9 +1353,6 @@ mod tests { KernelProcTable::new(), None, Trust::Refuse, - tokio::sync::broadcast::channel(8).0, - crate::stats::Stats::new(), - Default::default(), ) .err() .expect("a world-writable object must not be loaded"); @@ -2201,9 +1367,6 @@ mod tests { KernelProcTable::new(), None, Trust::Warn, - tokio::sync::broadcast::channel(8).0, - crate::stats::Stats::new(), - Default::default(), ) .err() .expect("`not an ELF` cannot load either way"); @@ -2226,9 +1389,6 @@ mod tests { KernelProcTable::new(), None, Trust::Refuse, - tokio::sync::broadcast::channel(8).0, - crate::stats::Stats::new(), - Default::default(), ) .err() .expect("there is no object there"); @@ -2532,9 +1692,6 @@ mod tests { KernelProcTable::new(), None, Trust::Warn, - tokio::sync::broadcast::channel(8).0, - crate::stats::Stats::new(), - Default::default(), ) .expect("the unit's capability set must be enough to load the object"); @@ -2572,37 +1729,33 @@ mod tests { "{} must be pinned alongside the verdict maps", sock_pids_pin.display() ); - // The fast path's maps and links. Unpinned, a restart would split - // them from the programs still attached (the SOCK_PIDS lesson), and - // an unpinned deadline map would be one no restarted daemon could - // ever refresh. + // The legacy Fast Allow maps stay pinned while the kernel object + // carries them (ABI v4): the startup disarm has to reach the maps the + // pinned programs read, and an unpinned copy would split them across + // a restart (the SOCK_PIDS lesson). for name in [ enforce::MAP_FAST_ALLOW, enforce::MAP_FAST_ALLOW_UNTIL, enforce::MAP_FAST_ALLOW_MARK, enforce::MAP_ALLOW_EVENTS, - enforce::LINK_SENDMSG4, - enforce::LINK_SENDMSG6, ] { let pin = enforce::pin_dir().join(name); assert!(pin.exists(), "{} must be pinned", pin.display()); } - // And the rung the fast path's safety argument stands on: the exec and - // exit tracepoint links pinned, not merely attached. + // The exec and exit tracepoint links pinned, not merely attached. // // Only the pin makes their clears outlive the daemon, and the connect // programs' links are pinned separately - so on a kernel where these - // two cannot be, the marking survives a dead daemon while the clearing - // does not. The loader used to throw the pin outcome away and the - // ladder could not see the difference; this is the assertion that - // stops it being thrown away again. Only this test can make it: the - // matrix guests have no bpffs. + // two cannot be, denials survive a dead daemon while their eviction + // does not. The loader used to throw the pin outcome away; this is + // the assertion that stops it being thrown away again. Only this test + // can make it: the matrix guests have no bpffs. for name in [enforce::LINK_EXEC, enforce::LINK_EXIT] { let pin = enforce::pin_dir().join(name); assert!( pin.exists(), - "{} must be pinned, or the fast path's clears die with the daemon", + "{} must be pinned, or eviction dies with the daemon", pin.display() ); } @@ -2611,9 +1764,68 @@ mod tests { "the report must say both lifecycle links pinned when they did: {:?}", report.notes ); + drop(attached); + + // The legacy disarm, on the path that needs it: a pinned MARK left + // armed by a 0.4-0.6 daemon that died, met by a restart on the + // inherited path with no engine. Only a bpffs host can show this. + // The fixture is inert while it sits there: a deadline already in + // the past (the hooks honour nothing) and a grant for a pid no + // process can hold. + { + let dir = enforce::pin_dir(); + let mark = MapData::from_pin(dir.join(enforce::MAP_FAST_ALLOW_MARK)) + .expect("reopen the pinned FAST_ALLOW_MARK"); + let mut mark = aya::maps::Array::<_, u32>::try_from(aya::maps::Map::Array(mark)) + .expect("FAST_ALLOW_MARK is an array"); + mark.set(0, 0x0003_3331, 0).expect("arm the legacy mark"); + let until = MapData::from_pin(dir.join(enforce::MAP_FAST_ALLOW_UNTIL)) + .expect("reopen the pinned FAST_ALLOW_UNTIL"); + let mut until = aya::maps::Array::<_, u64>::try_from(aya::maps::Map::Array(until)) + .expect("FAST_ALLOW_UNTIL is an array"); + until.set(0, 1, 0).expect("set a lapsed legacy deadline"); + let grants = MapData::from_pin(dir.join(enforce::MAP_FAST_ALLOW)) + .expect("reopen the pinned FAST_ALLOW"); + let mut grants = BpfHashMap::<_, u32, u32>::try_from(aya::maps::Map::HashMap(grants)) + .expect("FAST_ALLOW is a hash map"); + grants.insert(u32::MAX, 1, 0).expect("leave a legacy grant"); + } + let (attached, report) = load_and_attach( + Path::new(&path), + DnsCache::new(), + KernelProcTable::new(), + None, + Trust::Warn, + ) + .expect("the restart must load too"); + assert_eq!( + report.enforcement, + Enforcement::Inherited, + "{:?}", + report.notes + ); + { + let dir = enforce::pin_dir(); + let mark = MapData::from_pin(dir.join(enforce::MAP_FAST_ALLOW_MARK)).expect("mark"); + let mark = + aya::maps::Array::<_, u32>::try_from(aya::maps::Map::Array(mark)).expect("array"); + assert_eq!( + mark.get(&0, 0).expect("read"), + cfc_ebpf_common::fast_allow::UNARMED, + "a restart must disarm a legacy mark left in the pinned map" + ); + let until = MapData::from_pin(dir.join(enforce::MAP_FAST_ALLOW_UNTIL)).expect("until"); + let until = + aya::maps::Array::<_, u64>::try_from(aya::maps::Map::Array(until)).expect("array"); + assert_eq!(until.get(&0, 0).expect("read"), 0); + let grants = MapData::from_pin(dir.join(enforce::MAP_FAST_ALLOW)).expect("grants"); + let grants = + BpfHashMap::<_, u32, u32>::try_from(aya::maps::Map::HashMap(grants)).expect("hash"); + assert_eq!(grants.keys().count(), 0, "legacy grants must be emptied"); + } println!( - "seven programs attached; connect, sendmsg and lifecycle links plus \ - every fast-path map pinned, without CAP_SYS_ADMIN" + "five programs attached; connect and lifecycle links plus the \ + legacy maps pinned and disarmed, without CAP_SYS_ADMIN" ); drop(attached); @@ -2662,9 +1874,6 @@ mod tests { table.clone(), None, Trust::Warn, - tokio::sync::broadcast::channel(8).0, - crate::stats::Stats::new(), - Default::default(), ) .expect("load"); for note in &report.notes { @@ -2714,80 +1923,9 @@ mod tests { "cgroup/connect4|6 must load and attach on every matrix kernel: {:?}", report.notes ); - // The fast path's kernel side rides with the cookie connect variants, - // but not all the way: 5.10 verifies bpf_setsockopt on connect hooks - // and refuses it on sendmsg ones, so "cookie verified implies sendmsg - // verified" is false on the matrix floor and is not asserted. What is - // asserted is consistency: the two sendmsg programs verify together - // or not at all. - println!("fast_allow = {:?}", report.fast_allow); - let measured = |name: &str| report.verified_insns.iter().any(|(p, _)| p == name); - assert_eq!( - measured(enforce::PROG_SENDMSG4), - measured(enforce::PROG_SENDMSG6), - "the sendmsg pair must verify together or not at all: {:?}", - report.verified_insns - ); - // Off, and off for the reason this test's own setup dictates: it - // hands the layer no decision engine, so there is no sink to grant - // from. The first version of this assertion expected the config - // reason and learned on every matrix kernel at once that the test's - // inputs never reach the config check - an assertion about a state - // the test does not produce is the class of mistake this file exists - // to catch in the code, not to commit in the tests. - match &report.fast_allow { - Some(FastAllow::Off(why)) => assert!( - why.contains("no decision engine"), - "fast-allow is off for a reason this setup does not produce: {why}" - ), - other => panic!("fast-allow should be off here (no engine), got {other:?}"), - } - // The eligibility ladder's facts, observed on this kernel. The decision - // in this report is `Off` because the test hands the layer no engine, - // so the decision a rule set that grants would get is taken here from - // the facts the loader gathered - the ladder is a pure function with - // its own tests - and printed for the matrix summary. `config_on` and - // `has_maps` are this test's inputs, not observations: the object under - // test carries the maps, and the question is what the kernel would - // allow with the feature switched on. `lifecycle_pinned` is observed - // but says as much about the host as about the kernel: the qemu - // guests mount no bpffs, so it is false there on every kernel and - // the decision printed there carries the reduced deadline where a - // real host of the same kernel would pin. `exit_precise` and the - // capability are the kernel's own answers. - let capability = report - .fast_path_capability - .expect("enforcement is live, asserted above, so the capability is recorded"); - let facts = LadderFacts { - config_on: true, - has_maps: true, - exit_tracking: report.exit_tracking, - exit_precise: report.exit_precise, - lifecycle_pinned: report.lifecycle_pinned, - capability, - }; - let decision = fast_path_decision(&facts); - let described = match &decision { - Ok(reduced) if reduced.is_empty() => "live".to_string(), - Ok(reduced) => format!( - "live, grants lapse within {}s ({})", - deadline_pair(true).0, - reduced.join("; ") - ), - Err(why) => format!("off: {why}"), - }; println!( - "fast path on this kernel: {described} [exit_precise={} lifecycle_pinned={} capability={}]", - report.exit_precise, - report.lifecycle_pinned, - capability.as_str() - ); - // Runtime grants remain disabled regardless of the capabilities this - // kernel verifies. The matrix still records those facts for Deny and - // attribution compatibility; it must never claim Fast Allow is live. - assert!( - decision.is_err(), - "Fast Allow must be disabled on every matrix kernel: {decision:?}" + "lifecycle on this kernel: exit_precise={} lifecycle_pinned={}", + report.exit_precise, report.lifecycle_pinned ); // Where the matrix has already shown what a kernel answers, the answer // is asserted, so a kernel that changes its mind is caught here and not @@ -2795,26 +1933,10 @@ mod tests { // line printed above is what the first run of a new matrix entry // contributes to this table. match kernel_major_minor() { - Some((5, 10)) => { - assert!( - !report.exit_precise, - "5.10 has no group_dead in sched_process_exit" - ); - assert_eq!( - capability, - enforce::FastPathCapability::SendmsgUnavailable, - "5.10 takes the connect hooks and refuses bpf_getsockopt on the sendmsg ones" - ); - } - Some((5, 15)) | Some((6, 12)) => { + Some((5, 10)) | Some((5, 15)) | Some((6, 12)) => { assert!( !report.exit_precise, - "5.15 and 6.12 have no group_dead in sched_process_exit" - ); - assert_eq!( - capability, - enforce::FastPathCapability::Ready, - "5.15 already takes the sendmsg hooks 5.10 refuses, and so does 6.12" + "5.10, 5.15 and 6.12 have no group_dead in sched_process_exit" ); } Some((6, 18)) | Some((7, 1)) => { @@ -2822,18 +1944,11 @@ mod tests { report.exit_precise, "group_dead is in sched_process_exit from 6.18 on" ); - assert_eq!(capability, enforce::FastPathCapability::Ready); } other => { println!("kernel {other:?}: no recorded answer for this one, observation only") } } - // Where the sendmsg pair did not verify, the report must say so in the - // fast-path terms - the note is the only trace a kernel like 5.10 - // leaves, and it must not be mistaken for the basic-connect fallback. - if measured(enforce::PROG_CONNECT4) && !measured(enforce::PROG_SENDMSG4) { - println!("sendmsg hooks refused by this kernel; fast path unavailable here"); - } // `bpf_prog_info.verified_insns` exists since kernel 5.16; before // that, an empty report is the correct answer, not a recording // failure. On a kernel that does report counts, every program that diff --git a/crates/cfc-daemon/src/ebpf/nft_set.rs b/crates/cfc-daemon/src/ebpf/nft_set.rs index 69002d2..98ca840 100644 --- a/crates/cfc-daemon/src/ebpf/nft_set.rs +++ b/crates/cfc-daemon/src/ebpf/nft_set.rs @@ -1,78 +1,33 @@ -//! The one thing the daemon does to nftables: put its fast-allow mark into -//! the `fast_allow` set the snippet declares empty, and take it out again. +//! The daemon's two questions for nftables: is `table inet colony_firewall` +//! loaded, and, once at start, flush the legacy `fast_allow` set. //! -//! Until this module existed the daemon never wrote to the ruleset. It sat on -//! the far end of NFQUEUE 0 and `colony-firewall-nft.service` owned every -//! rule; that boundary was kept on purpose. It moves for exactly one reason, -//! given in full in [`cfc_ebpf_common::fast_allow`]: the mark the connect -//! hooks set must be a per-start random value, so it cannot be a literal in -//! the snippet, so something at runtime has to tell nftables what it is. This -//! is that something, and it is kept to two statements: `add element` when -//! the fast path comes up, `flush set` at every start and at shutdown. +//! The daemon does not write the ruleset. It sits on the far end of NFQUEUE 0 +//! and `colony-firewall-nft.service` owns every rule. The flush is the one +//! exception, and it is a removal: Fast Allow (0.4.0 to 0.6) put a per-start +//! random mark into that set, and a daemon that crashed while armed left it +//! there, accepted by a ruleset nothing reloaded. Fast Allow is gone; the +//! flush stays until no supported upgrade path starts from a release that had +//! it. //! //! # Why a child process //! //! `nft(8)` is run as a child, the way the provenance backend runs `rpm -qa`, //! and with the same discipline: a fixed program path, a deadline with a kill //! behind it, `LC_ALL=C`, stderr captured into the error and never parsed as -//! data. The alternative - speaking nf_tables netlink from the daemon - is a -//! batching protocol with its own cache semantics, for two statements the -//! package already `Requires: nftables` to make. One fork at start and one at -//! stop, both off the packet path. -//! -//! # What this module refuses to do -//! -//! Create the table or the set. Those belong to the snippet and its unit; a -//! daemon that made them on demand would be a daemon that quietly builds a -//! ruleset nobody loaded, and on a host without the snippet that ruleset -//! would be a fail-closed table with the wrong owner. A missing set is -//! reported as exactly that, with the fix. A missing table is reported as the -//! ordering it usually is: `colony-firewall-nft.service` is `After=` the -//! daemon, so at daemon start the table is normally not there *yet*, and at -//! stop (`PartOf=`) it is normally already gone. [`Absent`] tells the two -//! apart so the loader can retry the first, and [`disarm`] treats both as -//! nothing left to flush. -//! -//! # The value is a secret -//! -//! A forger holding `CAP_NET_RAW` but not `CAP_NET_ADMIN` can read neither the -//! ruleset nor the BPF map, and that is the whole argument for a random value. -//! The journal and `cfc status` are readable by more people than the ruleset -//! is, so the mark never appears in a log line or an error: nft echoes the -//! failing command back on stderr, and that echo is redacted before it goes -//! anywhere. -//! -//! There is a third read path, and naming two of them made this argument look -//! stronger than it is: a process the fast path has **granted** can read the -//! value straight off its own socket with `getsockopt(SO_MARK)`. Nothing -//! prevents that and nothing should - that process is allowed by a rule, which -//! is why the kernel marked it. What follows is the scope of the secret: it -//! holds against processes that have never been granted, and not against one -//! that has and then, say, execs into something a rule denies. The kernel -//! clears `FAST_ALLOW` on exec, but it cannot unmark a socket the old program -//! already passed on. The mark being redrawn at every daemon start is what -//! bounds that, and it is why [`disarm`] runs unconditionally at start rather -//! than only on the arming path - a value left accepted in the set that -//! nothing refreshes is one every past grantee still knows. - -// The only caller is the loader, which is behind the `ebpf` feature. The -// module itself stays in every build so its tests run in the default suite, -// for the same reason `tracefs` does. -#![cfg_attr(not(feature = "ebpf"), allow(dead_code))] +//! data. Speaking nf_tables netlink from the daemon would be a batching +//! protocol with its own cache semantics, for two commands the package +//! already `Requires: nftables` to make. One fork at start and one a minute +//! for the probe, both off the packet path. -use std::fmt; use std::io::Read as _; use std::path::Path; use std::process::{Command, ExitStatus, Stdio}; use std::time::{Duration, Instant}; -use anyhow::{anyhow, bail}; -use cfc_ebpf_common::fast_allow; +use anyhow::anyhow; use tracing::debug; -/// The family and table the snippet declares, and the set inside it. Named -/// once here and spelled into every command, so a rename in the snippet fails -/// the `list set` probe rather than silently arming nothing. +/// The family and table the snippet declares, and the legacy set inside it. const FAMILY: &str = "inet"; const TABLE: &str = "colony_firewall"; const SET: &str = "fast_allow"; @@ -88,135 +43,16 @@ const NFT_CANDIDATES: [&str; 2] = ["/usr/sbin/nft", "/usr/bin/nft"]; /// /// nft holds the nf_tables transaction lock for the length of its batch, and /// waits for it when another process - a large `nft -f`, a container runtime -/// rewriting its chains - holds it first. A daemon that hangs at start or -/// stop behind that lock is worse than one whose fast path stays off, and at -/// shutdown a hang here would run into the unit's stop timeout. Five seconds -/// is far past any healthy command and far short of that timeout. +/// rewriting its chains - holds it first. A daemon that hangs at start behind +/// that lock is worse than one whose legacy flush is logged as failed. Five +/// seconds is far past any healthy command and far short of the unit's +/// start timeout. const NFT_TIMEOUT: Duration = Duration::from_secs(5); /// How often the deadline is re-checked while waiting; same value and same /// reasoning as the rpm query's. const NFT_POLL_INTERVAL: Duration = Duration::from_millis(50); -/// Adds `mark` to `set fast_allow` in `table inet colony_firewall`. -/// -/// Errors when the set does not exist (a snippet that predates it), when the -/// table is not loaded, when `nft` is missing, or when the command fails; the -/// loader turns any error into `FastAllow::Off(reason)` and never arms the -/// kernel side without it. The first two are an [`Absent`], reachable through -/// `downcast_ref`, because one of them is the normal state right after -/// startup and deserves a retry rather than a reason. -/// -/// Refuses [`fast_allow::UNARMED`] outright: zero is the mark every socket -/// carries when nothing has marked it, and a zero element in the set would -/// accept every unmarked packet on the machine. -/// Serialises [`arm`] against the shutdown flush, and refuses an arm once that -/// flush has begun. -/// -/// Both run `nft`, and the heartbeat's arm runs inside `spawn_blocking`, which -/// `JoinHandle::abort` cannot cancel: aborting the heartbeat task leaves any -/// `nft add element` already in flight running to completion on the blocking -/// pool. So `Drop for Attached` could flush the set and *then* have that add -/// put the element back - leaving a mark accepted by the ruleset with no -/// daemon alive to refresh a deadline, honour a revocation, or ever remove it. -/// -/// The bool inside is "shutdown has started". Holding the lock across the -/// check and the command is what makes the two orders both end flushed: if the -/// heartbeat holds it, the shutdown flush waits and runs last; if the shutdown -/// flush holds it, the heartbeat then sees the flag and does not arm. -/// -/// It says "has started", not "has happened", so it belongs to one layer's -/// lifetime and [`disarm_for_start`] clears it. Leaving it set was a real bug -/// for as long as it existed: this is a process-global, so the first `Attached` -/// dropped would have refused every arm for the rest of that process - every -/// later test in one test binary, and any reload of the eBPF layer that did -/// not also restart the daemon. -static SHUTDOWN: std::sync::Mutex = std::sync::Mutex::new(false); - -/// Locks [`SHUTDOWN`], ignoring poisoning: the flag is a single bool that no -/// panic can leave inconsistent, and refusing to shut down cleanly because -/// some other thread panicked would be the worse failure. -fn shutdown_gate() -> std::sync::MutexGuard<'static, bool> { - SHUTDOWN.lock().unwrap_or_else(|e| e.into_inner()) -} - -/// Puts `mark` in the set, and only `mark`. -/// -/// Flushes first, so the set is left holding exactly this daemon's value -/// rather than this one added to whatever a predecessor left. Refuses once -/// [`disarm_for_shutdown`] has run - see [`SHUTDOWN`]. -pub(super) fn arm(mark: u32) -> anyhow::Result<()> { - if mark == fast_allow::UNARMED { - bail!( - "refusing to arm the fast path with mark {}: that is the mark of every \ - socket nothing has marked, and accepting it would accept everything", - mark_literal(mark) - ); - } - let gate = shutdown_gate(); - if *gate { - bail!("not arming the fast-allow set: the daemon is shutting down"); - } - arm_commands(mark, run) -} - -/// The ordered transaction steps, injectable without executing nft in tests. -fn arm_commands(mark: u32, mut run: impl FnMut(Op) -> Result<(), Failed>) -> anyhow::Result<()> { - match run(Op::ListSet) { - Ok(()) => {} - Err(failed) if failed.is_no_such_object() => { - // Table or set? nft says "No such file or directory" for both and - // only moves the caret. One more probe tells them apart, and it - // runs on this path alone. - let absent = match run(Op::ListTable) { - Ok(()) => Absent::Set, - Err(failed) if failed.is_no_such_object() => Absent::Table, - Err(failed) => return Err(failed.into_error(Op::ListTable)), - }; - return Err(anyhow::Error::new(absent)); - } - Err(failed) => return Err(failed.into_error(Op::ListSet)), - } - // Flush before adding, so this leaves the set holding *exactly* this - // daemon's mark rather than adding to whatever is already there. - // - // The startup flush is not enough on its own. It runs once, and on a boot - // it runs when the table does not exist yet - `colony-firewall-nft.service` - // is ordered after this daemon - so it flushes nothing. The heartbeat then - // retries this function on every heartbeat until the table appears, and a - // plain `add element` at that point would leave a crashed predecessor's - // mark accepted alongside this daemon's, for as long as the ruleset lives. - // A mark that is still accepted but that nothing refreshes is the worst - // shape this set can be in: every process that ever held it can read it - // back with `getsockopt(SO_MARK)` and set it again. - run(Op::FlushSet).map_err(|failed| failed.into_error(Op::FlushSet))?; - let op = Op::AddElement(mark); - run(op).map_err(|failed| failed.into_error(op))?; - debug!("fast-allow mark added to set {FAMILY} {TABLE} {SET}"); - Ok(()) -} - -/// Whether the ruleset still accepts `mark`. -/// -/// Arming is not a fact that stays true. `systemctl restart nftables`, a -/// `nft -f` that reloads the machine's ruleset, or anything else that -/// recreates `table inet colony_firewall` leaves the set empty while this -/// daemon goes on marking sockets and `cfc status` goes on saying `live`. The -/// heartbeat calls this so that state is noticed and re-armed rather than -/// reported. -/// -/// A missing table or set answers `false`, not an error: they are the same -/// answer for the caller - the mark is not accepted - and the caller's re-arm -/// path already classifies which of the two it is. -pub(super) fn holds(mark: u32) -> anyhow::Result { - let op = Op::GetElement(mark); - match run(op) { - Ok(()) => Ok(true), - Err(failed) if failed.is_no_such_object() => Ok(false), - Err(failed) => Err(failed.into_error(op)), - } -} - /// Whether `table inet colony_firewall` is loaded at all. /// /// This is the question `cfc status`'s `enforcing` is really asking. Without @@ -239,144 +75,43 @@ pub(super) fn table_loaded() -> anyhow::Result { } } -/// Flushes `set fast_allow`, so that no value (this daemon's or a previous -/// one's) is accepted by the ruleset. -/// -/// This is the plain flush: the late withdrawal in `load_and_attach` calls it -/// when the ring consumers failed after arming. The start and shutdown -/// flushes are [`disarm_for_start`] and [`disarm_for_shutdown`], which also -/// move the gate - an earlier version of this comment named them as this -/// function's callers, and they are not. +/// Flushes the legacy `set fast_allow`, so that no mark an older daemon left +/// there is accepted by the ruleset. /// /// A missing table or a missing set is success: there is nothing in either -/// that could accept a mark. The nft table intentionally remains loaded across -/// daemon restarts and stops; only an explicit nft-unit stop removes it. -pub(super) fn disarm() -> anyhow::Result<()> { - let _gate = shutdown_gate(); - flush() -} - -/// [`disarm`], beginning a new layer lifetime. -/// -/// For the one call at the top of `load_and_attach`. Clears the flag a -/// previous `Attached`'s shutdown set: that flag means "the layer that owned -/// this set is going away", and by here it has gone. -pub(super) fn disarm_for_start() -> anyhow::Result<()> { - begin_lifetime(); - flush() -} - -/// The flag half of [`disarm_for_start`], split out so the regression it -/// exists for can be tested without running `nft`. -fn begin_lifetime() { - *shutdown_gate() = false; -} - -/// [`disarm`], and no [`arm`] after it. -/// -/// For `Drop for Attached` only. Sets the flag the heartbeat's in-flight arm -/// will see, under the same lock, so the set cannot be re-armed behind a -/// daemon that has already stopped. -pub(super) fn disarm_for_shutdown() -> anyhow::Result<()> { - let mut gate = shutdown_gate(); - *gate = true; - flush() -} - -fn flush() -> anyhow::Result<()> { +/// that could accept a mark. At boot the table is normally not loaded yet, +/// because `colony-firewall-nft.service` is ordered after the daemon. +pub(super) fn flush() -> anyhow::Result<()> { match run(Op::FlushSet) { Ok(()) => { - debug!("fast-allow set flushed"); + debug!("legacy fast_allow set flushed"); Ok(()) } Err(failed) if failed.is_no_such_object() => { - debug!("fast-allow set not present, nothing to flush"); + debug!("legacy fast_allow set not present, nothing to flush"); Ok(()) } Err(failed) => Err(failed.into_error(Op::FlushSet)), } } -/// Why [`arm`] found nothing to arm. -/// -/// Returned as the error itself rather than as context, so the loader can -/// `downcast_ref::()` and treat the two differently: a missing table -/// is the expected state right after the daemon starts (the nft unit is -/// ordered after it) and is worth retrying; a missing set will not fix itself -/// and names its fix. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(super) enum Absent { - /// `table inet colony_firewall` is not loaded. - Table, - /// The table is loaded but carries no `fast_allow` set. - Set, -} - -impl fmt::Display for Absent { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Absent::Table => write!( - f, - "table {FAMILY} {TABLE} is not loaded, so there is no {SET} set to arm; \ - colony-firewall-nft.service loads it once the daemon is up" - ), - Absent::Set => write!( - f, - "the loaded nftables snippet predates {SET}; reinstall \ - systemd/nftables-snippet.conf and restart colony-firewall-nft.service" - ), - } - } -} - -impl std::error::Error for Absent {} - -/// The commands this module issues. Three do the work; `ListTable` exists -/// only to say which of two things is missing. +/// The commands this module issues. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum Op { - /// `nft list set inet colony_firewall fast_allow`: does the set exist? - /// Its output is discarded; the exit status is the answer. - ListSet, - /// `nft list table inet colony_firewall`, run only after `ListSet` failed. + /// `nft list table inet colony_firewall`: is the table loaded? Its output + /// is discarded; the exit status is the answer. ListTable, - /// `nft add element inet colony_firewall fast_allow { 0x }`. - AddElement(u32), /// `nft flush set inet colony_firewall fast_allow`. FlushSet, - /// `nft get element inet colony_firewall fast_allow { 0x }`: is - /// this daemon's mark still accepted? Status-only, like `ListSet`, which - /// is why it is a `get` and not a `list` the caller would have to parse. - GetElement(u32), } /// The argument vector for `op`, without the program. -/// -/// Pure, and the part of this module that is tested without nft: what the -/// tests pin is that every command names the snippet's table and set, and -/// that the mark is spelled one way. fn argv(op: Op) -> Vec { let words: &[&str] = match op { - Op::ListSet => &["list", "set", FAMILY, TABLE, SET], Op::ListTable => &["list", "table", FAMILY, TABLE], - Op::AddElement(_) => &["add", "element", FAMILY, TABLE, SET], Op::FlushSet => &["flush", "set", FAMILY, TABLE, SET], - Op::GetElement(_) => &["get", "element", FAMILY, TABLE, SET], }; - let mut argv: Vec = words.iter().map(|w| w.to_string()).collect(); - if let Op::AddElement(mark) | Op::GetElement(mark) = op { - argv.push(format!("{{ {} }}", mark_literal(mark))); - } - argv -} - -/// The mark as a `type mark` element: `0x` and exactly eight hex digits. -/// -/// One fixed spelling, because nft echoes the failing command line back on -/// stderr verbatim and [`redact`] removes the literal by exact match; a -/// literal that could be spelled two ways could be leaked one of them. -fn mark_literal(mark: u32) -> String { - format!("0x{mark:08x}") + words.iter().map(|w| w.to_string()).collect() } /// One nft command that did not succeed. @@ -397,13 +132,11 @@ impl Failed { matches!(self, Failed::Nft { stderr, .. } if stderr_names_no_such_object(stderr)) } - /// Folds into an error whose text is safe to log: the mark is redacted - /// from the command line and from nft's echo of it. fn into_error(self, op: Op) -> anyhow::Error { - let command = redact(op, format!("nft {}", argv(op).join(" "))); + let command = format!("nft {}", argv(op).join(" ")); match self { Failed::Nft { status, stderr } => { - anyhow!("{command} failed ({status}): {}", redact(op, stderr).trim()) + anyhow!("{command} failed ({status}): {}", stderr.trim()) } Failed::Run(e) => e.context(format!("running {command}")), } @@ -418,16 +151,6 @@ fn stderr_names_no_such_object(stderr: &str) -> bool { stderr.contains("No such file or directory") } -/// Replaces the mark literal with a placeholder. Only [`Op::AddElement`] and -/// [`Op::GetElement`] carry the mark; every other command's text is returned -/// as it is. -fn redact(op: Op, text: String) -> String { - match op { - Op::AddElement(mark) | Op::GetElement(mark) => text.replace(&mark_literal(mark), ""), - _ => text, - } -} - /// The first of [`NFT_CANDIDATES`] that exists. fn locate_nft() -> anyhow::Result<&'static str> { NFT_CANDIDATES @@ -506,89 +229,6 @@ fn run(op: Op) -> Result<(), Failed> { #[cfg(test)] mod tests { - - /// One test, not three: the flag is a process-global, so separate tests - /// touching it would race each other under the parallel harness. - /// - /// Neither half reaches `nft`. `arm` checks the gate before it runs - /// anything, and `begin_lifetime` is the flag half of `disarm_for_start` - /// split out for exactly this. - #[test] - fn a_shutdown_refuses_arming_until_a_new_layer_begins() { - let mark = 0x0003_3331; - - *shutdown_gate() = true; - let e = arm(mark).expect_err("an arm after shutdown must be refused"); - assert!( - e.to_string().contains("shutting down"), - "refused for the wrong reason: {e}" - ); - - // The regression: the flag was set by shutdown and cleared by nothing, - // so the first `Attached` dropped refused every arm for the rest of - // the process - every later test in one binary, and any reload of the - // layer that did not also restart the daemon. - begin_lifetime(); - assert!( - !*shutdown_gate(), - "a new layer lifetime must clear a previous shutdown's flag" - ); - - // Deliberately not "and now an arm succeeds": getting past the gate is - // the only thing left to check, and checking it means letting `arm` - // run nft - which on a machine that *does* have the table loaded would - // put a live element in a live ruleset from a unit test. The gate is - // two lines and the flag above is the whole of its state. - } - - #[test] - fn a_failed_flush_never_adds_a_mark() { - let mut attempted = Vec::new(); - let result = arm_commands(0x0003_3331, |op| { - attempted.push(op); - if op == Op::FlushSet { - Err(Failed::Run(anyhow!("flush refused"))) - } else { - Ok(()) - } - }); - assert!(result.is_err()); - assert_eq!(attempted, [Op::ListSet, Op::FlushSet]); - } - - /// The element check is status-only, like the set probe: `get element` - /// exits non-zero with ENOENT when the element is absent, so nothing here - /// has to parse nft's output. - #[test] - fn the_element_check_names_the_element_and_carries_the_mark() { - assert_eq!( - argv(Op::GetElement(0x0012_3456)), - vec![ - "get", - "element", - "inet", - "colony_firewall", - "fast_allow", - "{ 0x00123456 }", - ] - ); - } - - /// Both mark-carrying commands must be redacted, not just the add. nft - /// echoes the failing command line back verbatim, so a `get` that fails - /// would otherwise put the mark in the journal - where every reader of the - /// journal could set it. - #[test] - fn the_element_check_redacts_the_mark_from_what_nft_echoes() { - let mark = 0xdead_0a6c; - let echoed = format!("Error: No such file or directory\nget element inet colony_firewall fast_allow {{ {} }}", mark_literal(mark)); - let safe = redact(Op::GetElement(mark), echoed); - assert!( - !safe.contains(&mark_literal(mark)), - "leaked the mark: {safe}" - ); - assert!(safe.contains("")); - } use super::*; use std::os::unix::process::ExitStatusExt as _; @@ -597,7 +237,6 @@ mod tests { // moves, from under the table name to under the set name. const NO_TABLE_LIST: &str = "Error: No such file or directory\nlist set inet colony_firewall fast_allow\n ^^^^^^^^^^^^^^^\n"; const NO_SET_FLUSH: &str = "Error: No such file or directory\nflush set inet colony_firewall fast_allow\n ^^^^^^^^^^\n"; - const NO_SET_ADD: &str = "Error: No such file or directory\nadd element inet colony_firewall fast_allow { 0x1234abcd }\n ^^^^^^^^^^\n"; const SYNTAX_ERROR: &str = "Error: syntax error, unexpected newline\nexpected any of: , last\nlist set inet colony_firewall\n ^\n"; const NOT_PERMITTED: &str = "Error: Operation not permitted\nlist set inet colony_firewall fast_allow\n^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n"; @@ -606,46 +245,11 @@ mod tests { } #[test] - fn add_element_spells_the_mark_as_eight_hex_digits() { - assert_eq!( - argv(Op::AddElement(0x1234_abcd)), - [ - "add", - "element", - "inet", - "colony_firewall", - "fast_allow", - "{ 0x1234abcd }" - ] - ); - // A small value is padded rather than shortened: one spelling only, - // which is what makes the redaction an exact match. - assert_eq!(argv(Op::AddElement(7)).last().unwrap(), "{ 0x00000007 }"); - assert_eq!(mark_literal(u32::MAX), "0xffffffff"); - } - - #[test] - fn every_set_command_names_the_snippets_table_and_set() { - for op in [Op::ListSet, Op::AddElement(1), Op::FlushSet] { - let argv = argv(op); - assert_eq!( - &argv[2..5], - ["inet", "colony_firewall", "fast_allow"], - "{op:?} does not address the snippet's set" - ); - } + fn list_and_flush_use_the_verbs_nft_understands() { assert_eq!( argv(Op::ListTable), ["list", "table", "inet", "colony_firewall"] ); - } - - #[test] - fn list_and_flush_use_the_verbs_nft_understands() { - assert_eq!( - argv(Op::ListSet), - ["list", "set", "inet", "colony_firewall", "fast_allow"] - ); assert_eq!( argv(Op::FlushSet), ["flush", "set", "inet", "colony_firewall", "fast_allow"] @@ -654,7 +258,7 @@ mod tests { #[test] fn a_missing_table_and_a_missing_set_both_read_as_absent() { - for stderr in [NO_TABLE_LIST, NO_SET_FLUSH, NO_SET_ADD] { + for stderr in [NO_TABLE_LIST, NO_SET_FLUSH] { assert!( stderr_names_no_such_object(stderr), "not classified as absent:\n{stderr}" @@ -679,45 +283,4 @@ mod tests { let failed = Failed::Run(anyhow!("No such file or directory")); assert!(!failed.is_no_such_object()); } - - #[test] - fn the_unarmed_value_is_refused_before_nft_runs() { - // Zero is what every unmarked socket reads; accepting it would accept - // everything. This must fail on a machine without nft, which is why - // the guard comes before any command is built. - let e = arm(fast_allow::UNARMED).expect_err("mark 0 must be refused"); - assert!(e.to_string().contains("0x00000000"), "{e}"); - } - - #[test] - fn an_add_element_failure_never_names_the_mark() { - let mark = 0x1234_abcd; - let failed = Failed::Nft { - status: exit_status(1), - stderr: NO_SET_ADD.to_string(), - }; - let text = format!("{:#}", failed.into_error(Op::AddElement(mark))); - assert!(!text.contains("1234abcd"), "leaked: {text}"); - assert!(text.contains(""), "{text}"); - assert!(text.contains("exit status: 1"), "{text}"); - - let failed = Failed::Run(anyhow!("spawning /usr/sbin/nft: permission denied")); - let text = format!("{:#}", failed.into_error(Op::AddElement(mark))); - assert!(!text.contains("1234abcd"), "leaked: {text}"); - assert!(text.contains(""), "{text}"); - } - - #[test] - fn absent_names_its_fix_and_survives_the_error_chain() { - let set = Absent::Set.to_string(); - assert!(set.contains("nftables-snippet.conf"), "{set}"); - assert!(set.contains("colony-firewall-nft.service"), "{set}"); - let table = Absent::Table.to_string(); - assert!(table.contains("not loaded"), "{table}"); - assert!(table.contains("colony-firewall-nft.service"), "{table}"); - - // What the loader relies on to tell "retry" from "operator". - let e = anyhow::Error::new(Absent::Table); - assert_eq!(e.downcast_ref::(), Some(&Absent::Table)); - } } diff --git a/crates/cfc-daemon/src/ipc.rs b/crates/cfc-daemon/src/ipc.rs index da0db1f..3974121 100644 --- a/crates/cfc-daemon/src/ipc.rs +++ b/crates/cfc-daemon/src/ipc.rs @@ -880,9 +880,6 @@ impl Firewall for FirewallService { enforcement: crate::ebpf::enforcement_level() .map_or("starting", |l| l.as_str()) .to_string(), - fast_allow: crate::ebpf::fast_allow_level() - .map(|f| f.describe()) - .unwrap_or_default(), })) } diff --git a/crates/cfc-daemon/src/main.rs b/crates/cfc-daemon/src/main.rs index b00345d..7f260f5 100644 --- a/crates/cfc-daemon/src/main.rs +++ b/crates/cfc-daemon/src/main.rs @@ -41,8 +41,8 @@ const RUNTIME_SHUTDOWN_GRACE: Duration = Duration::from_secs(5); /// moved. /// How often to ask nftables whether the table that feeds NFQUEUE is loaded. /// -/// One minute, matching the fast-allow set check: both are a fork and an exec, -/// and both bound how long `cfc status` may be stale by the same amount. +/// One minute: it is a fork and an exec, and it bounds how long `cfc status` +/// may be stale. const NFT_PRESENCE_INTERVAL: std::time::Duration = std::time::Duration::from_secs(60); const PROVENANCE_WARM_INTERVAL: std::time::Duration = std::time::Duration::from_secs(120); @@ -289,10 +289,8 @@ async fn run() -> anyhow::Result<()> { // A packet counter cannot tell "nothing is filtered" from "nothing is // happening" - an idle laptop looks identical to an unprotected one. So // ask nftables instead. Once a minute, on the blocking pool because it is - // a fork and an exec, which is the same cadence and the same reasoning as - // the fast-allow set check that already runs there. An error leaves the - // previous answer standing: "could not ask" must never render as "the - // firewall is gone". + // a fork and an exec. An error leaves the previous answer standing: "could + // not ask" must never render as "the firewall is gone". { let stats = stats.clone(); tokio::spawn(async move { @@ -365,12 +363,12 @@ async fn run() -> anyhow::Result<()> { // sock_diag + /proc alone, which is exactly what the daemon does when the // layer is unavailable anyway. // - // The loader flushes a predecessor's fast-allow mark at the top of every - // load. With the layer switched off in the config that flush is never - // reached, and the nftables set outlives daemons - so it is done here for - // exactly that case. Not under --dry-run, which touches nothing. - if !args.dry_run && !cfg.ebpf.enabled.wants_load() { - ebpf::flush_stale_fast_allow(); + // Flush the legacy fast_allow nftables set before the layer comes up, + // whatever its mode and whether or not it is compiled in: a mark an older + // daemon left there would otherwise stay accepted. Not under --dry-run, + // which touches nothing. + if !args.dry_run { + ebpf::flush_legacy_fast_allow_set(); } // Held for the daemon's lifetime: dropping it detaches the programs. @@ -395,8 +393,6 @@ async fn run() -> anyhow::Result<()> { // direction of the dependency the same as everywhere else: the eBPF // layer is handed what it may read, and owns nothing the daemon needs. Some(engine.clone()), - observed_tx.clone(), - stats.clone(), ); _ebpf.report.log(); // Publish it so `cfc status` can say where enforcement lives without anyone diff --git a/crates/cfc-daemon/src/nfqueue.rs b/crates/cfc-daemon/src/nfqueue.rs index b7f2266..f119df1 100644 --- a/crates/cfc-daemon/src/nfqueue.rs +++ b/crates/cfc-daemon/src/nfqueue.rs @@ -240,6 +240,8 @@ trait PacketMessage { /// addresses - and guessing would be wrong on a multi-homed or routed /// host. This is what makes one queue able to serve both chains. fn hook(&self) -> u8; + /// Kernel output interface index; zero means it was not reported. + fn outdev(&self) -> u32; fn set_verdict(&mut self, verdict: NfqVerdict); } @@ -278,6 +280,10 @@ impl PacketMessage for Message { self.get_hook() } + fn outdev(&self) -> u32 { + self.get_outdev() + } + fn set_verdict(&mut self, verdict: NfqVerdict) { Message::set_verdict(self, verdict); } @@ -351,6 +357,15 @@ pub fn spawn( !cfg.fail_open, "NFQUEUE fail_open bypasses mandatory verdict auditing" ); + // Interface metadata, rather than destination addresses, includes every + // local host address routed over lo. A missing index cannot grant access. + // SAFETY: the C string is terminated and valid for this read-only query. + let loopback_ifindex = unsafe { libc::if_nametoindex(c"lo".as_ptr()) }; + anyhow::ensure!( + loopback_ifindex != 0, + "resolving loopback output interface: {}", + std::io::Error::last_os_error() + ); let queue_num = cfg.queue_num; info!(queue_num, "opening NFQUEUE"); @@ -408,6 +423,7 @@ pub fn spawn( let stop = Arc::new(AtomicBool::new(false)); let worker = Worker { queue, + loopback_ifindex, engine, rejecter, prompt_tx, @@ -533,6 +549,7 @@ impl Default for Tuning { /// for why the earlier blocking-when-idle mode had to go). struct Worker { queue: Q, + loopback_ifindex: u32, engine: Engine, /// Injects the TCP RST / ICMP port-unreachable that makes /// [`Action::Reject`] differ from [`Action::Deny`]. Inert (drop-only) @@ -717,6 +734,9 @@ impl Worker { uid: msg.uid(), gid: msg.gid(), direction: direction_for_hook(msg.hook()), + loopback: msg.hook() == NF_INET_LOCAL_OUT + && self.loopback_ifindex != 0 + && msg.outdev() == self.loopback_ifindex, }; let deps = PipelineDeps { engine: &self.engine, @@ -950,7 +970,7 @@ impl Worker { /// live /proc. trait ProcessResolver { #[allow(clippy::too_many_arguments)] // socket tuple plus kernel UID attribution - fn pid_for_socket( + fn socket_owner( &self, protocol: Protocol, direction: Direction, @@ -959,15 +979,15 @@ trait ProcessResolver { dst_ip: IpAddr, dst_port: u16, uid: Option, - ) -> Option; - fn resolve(&self, pid: u32) -> Process; + ) -> Option; + fn resolve(&self, owner: &process_resolve::SocketOwner) -> Process; } /// Production resolver backed by /proc. struct ProcfsResolver; impl ProcessResolver for ProcfsResolver { - fn pid_for_socket( + fn socket_owner( &self, protocol: Protocol, direction: Direction, @@ -976,14 +996,12 @@ impl ProcessResolver for ProcfsResolver { dst_ip: IpAddr, dst_port: u16, uid: Option, - ) -> Option { - process_resolve::pid_for_socket( - protocol, direction, src_ip, src_port, dst_ip, dst_port, uid, - ) + ) -> Option { + process_resolve::socket_owner(protocol, direction, src_ip, src_port, dst_ip, dst_port, uid) } - fn resolve(&self, pid: u32) -> Process { - process_resolve::resolve(pid) + fn resolve(&self, owner: &process_resolve::SocketOwner) -> Process { + owner.resolve() } } @@ -1021,6 +1039,8 @@ struct PacketMeta { gid: Option, /// Which way this packet is going, from the netfilter hook. direction: Direction, + /// Kernel OUTPUT interface is lo; includes local non-loopback addresses. + loopback: bool, } /// Environment for [`handle_packet`]. @@ -1081,7 +1101,7 @@ fn handle_packet(payload: &[u8], meta: &PacketMeta, deps: &PipelineDeps) -> Pack // Inbound is not asked, rather than asked and told nothing. // - // `pid_for_socket` searches for a socket already holding this 4-tuple. + // `socket_owner` searches for a socket already holding this 4-tuple. // An inbound SYN has none - nothing has accepted it yet - so the search // could only ever miss, and missing meant reading /proc/net/tcp and // /proc/net/tcp6: 2.40 ms per packet for an answer of `None`. @@ -1090,10 +1110,10 @@ fn handle_packet(payload: &[u8], meta: &PacketMeta, deps: &PipelineDeps) -> Pack // one is silent: the verdict is identical either way, so nothing would // have failed - the firewall would just be fourteen times slower on the // inbound side and say nothing about it. - let pid_hint = if conn.direction == Direction::Inbound { + let owner = if conn.direction == Direction::Inbound { None } else { - deps.resolver.pid_for_socket( + deps.resolver.socket_owner( conn.protocol, conn.direction, conn.src_ip, @@ -1103,6 +1123,7 @@ fn handle_packet(payload: &[u8], meta: &PacketMeta, deps: &PipelineDeps) -> Pack meta.uid, ) }; + let pid_hint = owner.as_ref().map(process_resolve::SocketOwner::pid); // Always allow our own traffic. Otherwise the daemon's reverse DNS // resolver would itself be intercepted, deadlocking on a verdict @@ -1113,8 +1134,8 @@ fn handle_packet(payload: &[u8], meta: &PacketMeta, deps: &PipelineDeps) -> Pack } } - let mut proc = match pid_hint { - Some(pid) => deps.resolver.resolve(pid), + let mut proc = match owner.as_ref() { + Some(owner) => deps.resolver.resolve(owner), None => Process::unknown(0), }; // The kernel-reported socket uid/gid come from the sk_buff itself and @@ -1174,6 +1195,15 @@ fn handle_packet(payload: &[u8], meta: &PacketMeta, deps: &PipelineDeps) -> Pack verdict, }; } + // Preserve desktop IPC for unmatched local flows after explicit + // policy and incomplete-identity refusals have had their say. + if meta.loopback { + return PacketOutcome::Deliver { + connection: conn, + process: proc, + verdict: Verdict::default_allow(), + }; + } if deps.stats.is_paused() { // Paused means "stop prompting", not "stop filtering": // rules above still applied; only unmatched flows pass @@ -1291,7 +1321,7 @@ mod tests { } impl ProcessResolver for StubResolver { - fn pid_for_socket( + fn socket_owner( &self, _protocol: Protocol, _direction: Direction, @@ -1300,13 +1330,13 @@ mod tests { _dst_ip: IpAddr, _dst_port: u16, _uid: Option, - ) -> Option { + ) -> Option { self.socket_lookups .fetch_add(1, std::sync::atomic::Ordering::Relaxed); - self.pid + self.pid.map(process_resolve::SocketOwner::for_test) } - fn resolve(&self, _pid: u32) -> Process { + fn resolve(&self, _owner: &process_resolve::SocketOwner) -> Process { self.process.clone() } } @@ -1346,6 +1376,7 @@ mod tests { uid: None, gid: None, direction: Direction::Outbound, + loopback: false, }; /// The same, for a packet the kernel queued from the input hook. @@ -1353,6 +1384,7 @@ mod tests { uid: None, gid: None, direction: Direction::Inbound, + loopback: false, }; struct TestEnv { @@ -1960,7 +1992,7 @@ mod tests { let meta = PacketMeta { uid: Some(0), gid: Some(0), - direction: Direction::Outbound, + ..NO_META }; match env.handle(&tcp_packet(443), &meta) { PacketOutcome::Prompt { @@ -1984,7 +2016,7 @@ mod tests { let meta = PacketMeta { uid: Some(1000), gid: None, - direction: Direction::Outbound, + ..NO_META }; match env.handle(&tcp_packet(443), &meta) { PacketOutcome::Prompt { @@ -2189,6 +2221,7 @@ mod tests { uid: Option, gid: Option, hook: u8, + outdev: u32, verdict: Option, } @@ -2200,12 +2233,17 @@ mod tests { uid: None, gid: None, hook: NF_INET_LOCAL_OUT, + outdev: 0, verdict: None, } } } impl PacketMessage for FakeMsg { + fn outdev(&self) -> u32 { + self.outdev + } + fn hook(&self) -> u8 { self.hook } @@ -2324,6 +2362,7 @@ mod tests { script: script.into(), log: log.clone(), }, + loopback_ifindex: 1, engine: Engine::new(RuleSet { rules }, Arc::new(std::sync::RwLock::new(policy))), rejecter: Rejecter::open(), prompt_tx, @@ -2555,6 +2594,106 @@ mod tests { assert_eq!(h.stats.connections_denied(), 2); } + #[test] + fn loopback_unmatched_flows_keep_the_nonprompting_local_default() { + // The output interface, including local host addresses, defines this + // exception. IPv4 loopback, IPv6 loopback, and a local host address + // must all retain the same desktop IPC behavior. + let mut ipv4 = tcp_packet(53); + ipv4[12..16].copy_from_slice(&[127, 0, 0, 1]); + ipv4[16..20].copy_from_slice(&[127, 0, 0, 53]); + let mut ipv6 = vec![0u8; 44]; + ipv6[0] = 0x60; + ipv6[6] = 6; + ipv6[23] = 1; + ipv6[39] = 1; + ipv6[40..42].copy_from_slice(&5555u16.to_be_bytes()); + ipv6[42..44].copy_from_slice(&53u16.to_be_bytes()); + for payload in [ipv4, ipv6, tcp_packet(53)] { + let mut h = LoopHarness::new(vec![], vec![], dp_deny()); + let mut msg = FakeMsg::new(1, payload); + msg.outdev = 1; + h.worker().handle_message(msg).unwrap(); + assert_eq!(h.verdicts(), vec![(1, NfqVerdict::Accept)]); + assert!(h.prompt_rx.try_recv().is_err(), "local IPC must not prompt"); + assert_eq!(h.stats.connections_allowed(), 1); + } + } + + #[test] + fn loopback_addresses_without_the_local_output_interface_still_prompt() { + for outdev in [0, 2] { + let mut h = LoopHarness::new(vec![], vec![], dp_deny()); + let mut payload = tcp_packet(53); + payload[16..20].copy_from_slice(&[127, 0, 0, 53]); + let mut msg = FakeMsg::new(1, payload); + msg.outdev = outdev; + h.worker().handle_message(msg).unwrap(); + assert!(h.verdicts().is_empty()); + assert!(h.prompt_rx.try_recv().is_ok()); + } + } + + #[test] + fn loopback_closed_rules_are_audited_before_release_even_when_paused() { + for action in [Action::Deny, Action::Reject] { + let mut scope = RuleScope::any(); + scope.exe_path = Some(PathBuf::from("/usr/bin/curl")); + let rule = Rule::new("local application refusal", action, scope); + let store = RuleStore::open_in_memory().unwrap(); + let mut h = LoopHarness::new(vec![], vec![rule], dp_deny()).with_store(store); + h.stats.set_paused(true); + let mut payload = tcp_packet(53); + // Policy needs only ports. No complete TCP header keeps refusal + // injection inert while testing the real verdict and audit gate. + payload.truncate(24); + let mut msg = FakeMsg::new(1, payload); + msg.outdev = 1; + h.worker().handle_message(msg).unwrap(); + assert_eq!(h.verdicts(), vec![(1, NfqVerdict::Drop)]); + assert_eq!(h.log.lock().unwrap().audited_at_verdict, vec![1]); + assert_eq!(h.observed_rx.try_recv().unwrap().verdict.action, action); + assert!(h.prompt_rx.try_recv().is_err()); + } + } + + #[test] + fn loopback_missing_identity_cannot_override_an_application_refusal() { + let mut scope = RuleScope::any(); + scope.exe_path = Some(PathBuf::from("/usr/bin/curl")); + let rule = Rule::new("local application refusal", Action::Deny, scope); + let store = RuleStore::open_in_memory().unwrap(); + let mut h = LoopHarness::new(vec![], vec![rule], dp_deny()).with_store(store); + h.worker().resolver = Box::new(StubResolver { + pid: None, + process: Process::unknown(0), + socket_lookups: std::sync::atomic::AtomicUsize::new(0), + }); + h.stats.set_paused(true); + let mut msg = FakeMsg::new(1, tcp_packet(53)); + msg.outdev = 1; + h.worker().handle_message(msg).unwrap(); + assert_eq!(h.verdicts(), vec![(1, NfqVerdict::Drop)]); + assert_eq!(h.log.lock().unwrap().audited_at_verdict, vec![1]); + assert!(h.prompt_rx.try_recv().is_err()); + } + + #[test] + fn loopback_keeps_the_root_daemon_dns_exception() { + let mut h = LoopHarness::new(vec![], vec![deny_port_rule(53)], dp_deny()); + h.worker().dns = Box::new(StubDns { + self_pid: Some(4242), + ..Default::default() + }); + let mut msg = FakeMsg::new(1, tcp_packet(53)); + msg.outdev = 1; + msg.uid = Some(0); + h.worker().handle_message(msg).unwrap(); + assert_eq!(h.verdicts(), vec![(1, NfqVerdict::Accept)]); + assert!(h.prompt_rx.try_recv().is_err()); + assert_eq!(h.stats.connections_total(), 0); + } + #[test] fn allowed_delivery_enriches_and_counts_once_without_a_refusal_audit() { struct CountingDns(Arc); diff --git a/crates/cfc-daemon/src/process_resolve.rs b/crates/cfc-daemon/src/process_resolve.rs index ff9b5a3..2f10a78 100644 --- a/crates/cfc-daemon/src/process_resolve.rs +++ b/crates/cfc-daemon/src/process_resolve.rs @@ -9,7 +9,8 @@ //! unconnected-UDP, wildcard-bind, v4-mapped-in-v6). //! 3. inode -> pid via a verified TTL cache, else a /proc/*/fd walk. //! -//! TOCTOU note: the resolved pid may have exited by the time we describe it. +//! Socket ownership retains the process generation and descriptor across +//! image reads. A changed generation or closed descriptor leaves identity unknown. //! Process identity is read on every resolve: exec preserves pid and starttime. //! The inode cache re-verifies its answer with a single readlink before //! trusting it. @@ -98,6 +99,52 @@ pub fn resolve(pid: u32) -> Process { } } +/// Descriptor ownership carried across process-image resolution. This +/// establishes a current holder, not the process that sent a queued packet. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SocketOwner { + pid: u32, + starttime: u64, + inode: u64, + fd: i32, +} + +impl SocketOwner { + pub fn pid(&self) -> u32 { + self.pid + } + + pub fn resolve(&self) -> Process { + if read_starttime(self.pid) != Some(self.starttime) { + return Process::unknown(self.pid); + } + let process = resolve_inner( + self.pid, + Some(self.starttime), + Instant::now(), + crate::ebpf::proc_table::global(), + ) + .unwrap_or_else(|_| Process::unknown(self.pid)); + if fd_points_at_socket(self.pid, self.fd, self.inode) + && read_starttime(self.pid) == Some(self.starttime) + { + process + } else { + Process::unknown(self.pid) + } + } + + #[cfg(test)] + pub(crate) fn for_test(pid: u32) -> Self { + Self { + pid, + starttime: 0, + inode: 0, + fd: -1, + } + } +} + /// The table is a parameter rather than a reach into /// `crate::ebpf::proc_table::global()` so the tests below can drive both /// branches without mutating process-wide state that every other test in the @@ -225,7 +272,7 @@ fn resolve_inner( }) } -/// Find the pid that owns a socket matching the given 5-tuple. +/// Find a current descriptor holder for a socket matching the given 5-tuple. /// /// TCP tries sock_diag first. UDP requires a unique inode across all relevant /// tables before a diagnostic cookie or an fd walk may identify the owner. @@ -255,7 +302,7 @@ fn resolve_inner( /// flow to the process *listening* on the port is a real and separate thing, /// and it would be one netlink round trip rather than two /proc scans; it is /// not done here because it would change which rules match, not just how fast. -pub fn pid_for_socket( +pub fn socket_owner( protocol: Protocol, direction: Direction, src_ip: IpAddr, @@ -263,7 +310,7 @@ pub fn pid_for_socket( dst_ip: IpAddr, dst_port: u16, uid: Option, -) -> Option { +) -> Option { if direction == Direction::Inbound { return None; } @@ -287,24 +334,16 @@ pub fn pid_for_socket( .filter(|info| uid.is_none_or(|uid| info.uid == uid)) .filter(|info| udp_inode.is_none_or(|inode| info.inode == inode)); - // Fastest path: the kernel recorded cookie -> tgid at connect() time - // (`SOCK_PIDS`, written by cfc_connect4|6 in the connecting process's own - // context). One map lookup replaces the /proc walk below, which measures - // 37-44 ms on a loaded desktop - per NEW connection, before rule - // evaluation, on the only worker thread. This one line is the difference - // between the firewall being invisible and being felt. - if let Some(cookie) = info.as_ref().and_then(|i| i.cookie) { - if let Some(pid) = crate::ebpf::cookie_pid(cookie) { - record_resolved_pid(pid); - return Some(pid); - } - } + let cookie_pid = info + .as_ref() + .and_then(|i| i.cookie) + .and_then(crate::ebpf::cookie_pid); let inode = udp_inode .or_else(|| info.map(|i| i.inode)) .or_else(|| proc_net_inode(protocol, src_ip, src_port, dst_ip, dst_port, uid, deadline))?; - pid_owning_inode(inode, deadline) + pid_owning_inode(inode, cookie_pid, deadline) } /// Pids that recently owned a resolved socket, most recent first. @@ -332,15 +371,30 @@ fn record_resolved_pid(pid: u32) { /// /// The unit of work the probe lists reuse: one process's fd table instead of /// every process's. -fn pid_has_socket_inode(pid: u32, inode: u64) -> Option { +fn pid_has_socket_inode(pid: u32, inode: u64, deadline: Instant) -> Option { + if inode == 0 || Instant::now() >= deadline { + return None; + } let p = ProcFsProcess::new(pid as i32).ok()?; + let starttime = read_starttime(pid)?; let fds = p.fd().ok()?; for fd in fds.flatten() { + if Instant::now() >= deadline { + return None; + } if matches!(fd.target, FDTarget::Socket(i) if i == inode) { + if read_starttime(pid) != Some(starttime) { + return None; + } INODE_PID_CACHE .lock() .insert(inode, (pid, fd.fd), Instant::now()); - return Some(pid); + return Some(SocketOwner { + pid, + starttime, + inode, + fd: fd.fd, + }); } } None @@ -430,21 +484,16 @@ fn udp_inode_from_tables( struct TableEntry { local: (IpAddr, u16), remote: (IpAddr, u16), + state: u8, inode: u64, uid: u32, } /// Match a socket table (the text of /proc/net/{tcp,udp}{,6}) against a /// flow. UDP requires one unique compatible inode across all match classes. -/// TCP uses decreasing precision and stops at the first hit. -/// -/// Pass 1 - exact local+remote: connected TCP/UDP sockets. -/// Pass 2 - UDP only, exact local, zero remote: unconnected UDP sockets -/// doing plain sendto() (mDNS, NTP, syslog, QUIC stacks) list their -/// remote as 0.0.0.0:0, so an exact-remote match can never hit them. -/// Pass 3 - wildcard local addr, matching port, remote exact-or-zero: -/// sockets bound to 0.0.0.0 / :: show the wildcard, not the address -/// the flow actually uses. +/// TCP requires an exact connected tuple and never selects a listener. +/// UDP also includes zero-remote and wildcard-local sockets for sendto() +/// users (mDNS, NTP, syslog, QUIC); every compatible inode must agree. /// /// All address comparisons canonicalize v4-mapped v6 (::ffff:a.b.c.d) to /// plain v4 first, which is how dual-stack sockets appear in the v6 tables. @@ -493,19 +542,11 @@ fn scan_table_entries( return inode; } - // Pass 1: exact 4-tuple. - for e in entries.clone() { - if endpoint_eq(e.local, local) && endpoint_eq(e.remote, remote) { - return Some(e.inode); - } - } - - // Pass 3: wildcard-bound local (port must match), remote exact or zero - // (zero covers listeners and wildcard-bound unconnected UDP). for e in entries { - if e.local.1 == local.1 - && e.local.0.to_canonical().is_unspecified() - && (endpoint_eq(e.remote, remote) || endpoint_is_zero(e.remote)) + if e.state != 0x0A // TCP_LISTEN + && !endpoint_is_zero(e.remote) + && endpoint_eq(e.local, local) + && endpoint_eq(e.remote, remote) { return Some(e.inode); } @@ -527,7 +568,7 @@ fn parse_table_line(line: &str) -> Option { let _sl = cols.next()?; let local = parse_hex_addr_port(cols.next()?)?; let remote = parse_hex_addr_port(cols.next()?)?; - let _state = cols.next()?; + let state = u8::from_str_radix(cols.next()?, 16).ok()?; let _txrx = cols.next()?; let _tr = cols.next()?; let _retr = cols.next()?; @@ -537,6 +578,7 @@ fn parse_table_line(line: &str) -> Option { Some(TableEntry { local, remote, + state, inode, uid, }) @@ -600,12 +642,33 @@ fn format_addr_port(ip: IpAddr, port: u16) -> String { /// A verified cache fronts the /proc/*/fd walk: on a hit we re-readlink /// the remembered fd and only trust the pid if it still points at /// `socket:[inode]`; otherwise the entry is dropped and we re-walk. -fn pid_owning_inode(inode: u64, deadline: Instant) -> Option { +fn pid_owning_inode(inode: u64, cookie_pid: Option, deadline: Instant) -> Option { + if inode == 0 || Instant::now() >= deadline { + return None; + } + // The connect-time cookie records a numeric PID, not its lifetime or + // current descriptor ownership. It is only a hint for the verified walk. + if let Some(pid) = cookie_pid { + if let Some(found) = pid_has_socket_inode(pid, inode, deadline) { + record_resolved_pid(found.pid); + return Some(found); + } + } let now = Instant::now(); let cached = INODE_PID_CACHE.lock().get(&inode, now); if let Some((pid, fd)) = cached { - if fd_points_at_socket(pid, fd, inode) { - return Some(pid); + let starttime = read_starttime(pid); + if fd_points_at_socket(pid, fd, inode) && Instant::now() < deadline { + if let Some(starttime) = + starttime.filter(|starttime| read_starttime(pid) == Some(*starttime)) + { + return Some(SocketOwner { + pid, + starttime, + inode, + fd, + }); + } } INODE_PID_CACHE.lock().remove(&inode); } @@ -625,15 +688,15 @@ fn pid_owning_inode(inode: u64, deadline: Instant) -> Option { // (16 entries against 24), which makes the miss cheaper as well. let recently_resolved: Vec = RESOLVED_PIDS.lock().iter().copied().collect(); for pid in recently_resolved { - if let Some(found) = pid_has_socket_inode(pid, inode) { - record_resolved_pid(found); + if let Some(found) = pid_has_socket_inode(pid, inode, deadline) { + record_resolved_pid(found.pid); return Some(found); } } let recent_execs = crate::ebpf::proc_table::global().recent_pids(24, Instant::now()); for pid in recent_execs { - if let Some(found) = pid_has_socket_inode(pid, inode) { - record_resolved_pid(found); + if let Some(found) = pid_has_socket_inode(pid, inode, deadline) { + record_resolved_pid(found.pid); return Some(found); } } @@ -652,8 +715,8 @@ fn pid_owning_inode(inode: u64, deadline: Instant) -> Option { if Instant::now() > deadline { return None; } - if let Some(found) = pid_has_socket_inode(pid, inode) { - record_resolved_pid(found); + if let Some(found) = pid_has_socket_inode(pid, inode, deadline) { + record_resolved_pid(found.pid); return Some(found); } } @@ -952,6 +1015,52 @@ mod tests { // -- table scanning --------------------------------------------------- + #[test] + fn cookie_pid_hint_requires_live_socket_ownership() { + use std::os::fd::AsRawFd; + use std::os::unix::net::UnixDatagram; + + let pid = std::process::id(); + let socket = UnixDatagram::unbound().unwrap(); + let link = fs::read_link(format!("/proc/self/fd/{}", socket.as_raw_fd())).unwrap(); + let inode = link + .to_str() + .unwrap() + .strip_prefix("socket:[") + .unwrap() + .strip_suffix(']') + .unwrap() + .parse() + .unwrap(); + let budget = || Instant::now() + Duration::from_secs(2); + let owner = pid_owning_inode(inode, Some(pid), budget()).unwrap(); + assert_eq!(owner.pid(), pid); + assert_ne!(owner.resolve().exe.to_str(), Some(cfc_core::UNKNOWN_EXE)); + assert_eq!(pid_owning_inode(u64::MAX, Some(pid), budget()), None); + // A dead hint must not mask the descriptor's current holder. + assert_eq!( + pid_owning_inode(inode, Some(u32::MAX), budget()).map(|o| o.pid()), + Some(pid) + ); + assert_eq!(pid_owning_inode(0, Some(pid), budget()), None); + assert_eq!( + pid_owning_inode(inode, Some(pid), Instant::now() - Duration::from_secs(1)), + None + ); + // Model a different process generation at the validation-to-use edge. + let changed_generation = SocketOwner { + starttime: owner.starttime + 1, + ..owner + }; + assert_eq!( + changed_generation.resolve().exe.to_str(), + Some(cfc_core::UNKNOWN_EXE) + ); + // A closed descriptor cannot authorize a subsequently read image. + drop(socket); + assert_eq!(owner.resolve().exe.to_str(), Some(cfc_core::UNKNOWN_EXE)); + } + const HEADER: &str = " sl local_address rem_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode\n"; @@ -994,9 +1103,7 @@ mod tests { #[test] fn unconnected_fallback_is_udp_only() { - // The zero-remote pass must not apply to TCP: a TCP row with a - // zero remote is a listener, matched (if at all) by the wildcard - // pass, not by pretending it is connected to our destination. + // The zero-remote UDP match must not attribute TCP to a listener. let local = v4(10, 0, 2, 15, 5353); let table = format!("{HEADER}{}", line(local, v4(0, 0, 0, 0, 0), "0A", 4242)); assert_eq!( @@ -1026,8 +1133,8 @@ mod tests { } #[test] - fn wildcard_v6_matches_v4_flow() { - // Dual-stack socket bound to [::]:8080 must attribute v4 traffic. + fn outbound_tcp_cannot_borrow_a_dual_stack_listener() { + // An outbound flow must not inherit a listening application's policy. let local = (IpAddr::V6(Ipv6Addr::UNSPECIFIED), 8080); let remote = (IpAddr::V6(Ipv6Addr::UNSPECIFIED), 0); let table = format!("{HEADER}{}", line(local, remote, "0A", 909)); @@ -1039,7 +1146,28 @@ mod tests { v4(1, 2, 3, 4, 55000), None ), - Some(909) + None + ); + } + + #[test] + fn outbound_tcp_requires_a_connected_exact_tuple() { + let local = v4(10, 0, 0, 7, 8080); + let remote = v4(1, 2, 3, 4, 55000); + for listener_local in [local, v4(0, 0, 0, 0, 8080)] { + let table = format!( + "{HEADER}{}", + line(listener_local, v4(0, 0, 0, 0, 0), "0A", 909) + ); + assert_eq!( + scan_table_content(&table, Protocol::Tcp, local, remote, Some(1000)), + None + ); + } + let table = format!("{HEADER}{}", line(local, remote, "0A", 909)); + assert_eq!( + scan_table_content(&table, Protocol::Tcp, local, remote, Some(1000)), + None ); } diff --git a/crates/cfc-daemon/src/provenance.rs b/crates/cfc-daemon/src/provenance.rs index 7b69bcd..8d86a65 100644 --- a/crates/cfc-daemon/src/provenance.rs +++ b/crates/cfc-daemon/src/provenance.rs @@ -1806,7 +1806,11 @@ mod tests { (cold, incl. index build: {:?})", started.elapsed() ); - assert_eq!(package.as_deref(), Some("curl 8.21.0-1")); + // The owning package, not a version: curl updates under this test. + assert!( + package.as_deref().is_some_and(|p| p.starts_with("curl ")), + "/usr/bin/curl must belong to the curl package, got {package:?}" + ); assert_eq!( provenance, Provenance::Verified, @@ -1826,10 +1830,7 @@ mod tests { // the file the kernel mapped is not the file the package shipped. let tampered = describe(curl, Some(&"0".repeat(64))); println!("/usr/bin/curl with a foreign digest -> {tampered:?}"); - assert_eq!( - tampered, - (Some("curl 8.21.0-1".to_string()), Provenance::Modified) - ); + assert_eq!(tampered, (package.clone(), Provenance::Modified)); // A byte-identical copy in /tmp is owned by nobody: the dropper case. let tmp = tempfile::tempdir().unwrap(); diff --git a/crates/cfc-daemon/src/sock_diag.rs b/crates/cfc-daemon/src/sock_diag.rs index 7e54a36..4032c55 100644 --- a/crates/cfc-daemon/src/sock_diag.rs +++ b/crates/cfc-daemon/src/sock_diag.rs @@ -210,13 +210,13 @@ fn reply_seq(buf: &[u8]) -> Option { } /// answer with a single SOCK_DIAG_BY_FAMILY message or an NLMSG_ERROR. -fn parse_response(buf: &[u8]) -> Option { +fn parse_response(buf: &[u8], protocol: u8) -> Option { if buf.len() < NLMSG_HDR_LEN { return None; } let msg_len = u32::from_ne_bytes(buf[0..4].try_into().ok()?) as usize; let msg_type = u16::from_ne_bytes(buf[4..6].try_into().ok()?); - if msg_type != SOCK_DIAG_BY_FAMILY || msg_len > buf.len() { + if msg_type != SOCK_DIAG_BY_FAMILY || msg_len < NLMSG_HDR_LEN || msg_len > buf.len() { // NLMSG_ERROR (no such socket, EPERM, ...) or truncated reply. return None; } @@ -224,6 +224,11 @@ fn parse_response(buf: &[u8]) -> Option { if payload.len() < INET_DIAG_MSG_LEN { return None; } + // A listener is not the connected socket that emitted an outbound flow. + // UDP's unconnected state remains valid and is checked by the caller. + if protocol == libc::IPPROTO_TCP as u8 && payload[1] == 0x0A { + return None; + } // struct inet_diag_msg: id.idiag_cookie sits at payload offset 44 // (family/state/timer/retrans = 4, sport+dport = 4, src = 16, dst = 16, // if = 4), then expires, rqueue, wqueue, uid at 64, inode at 68. The @@ -323,7 +328,7 @@ impl DiagSocket { trace!("sock_diag answered a different request; discarding the socket"); return Reply::Desync; } - match parse_response(buf) { + match parse_response(buf, req[17]) { Some(info) => Reply::Found(info), None => Reply::NotFound, } @@ -442,7 +447,7 @@ mod tests { buf[NLMSG_HDR_LEN + 64..NLMSG_HDR_LEN + 68].copy_from_slice(&1000u32.to_ne_bytes()); buf[NLMSG_HDR_LEN + 68..NLMSG_HDR_LEN + 72].copy_from_slice(&31337u32.to_ne_bytes()); assert_eq!( - parse_response(&buf), + parse_response(&buf, libc::IPPROTO_TCP as u8), Some(SockInfo { inode: 31337, cookie: None, @@ -451,6 +456,27 @@ mod tests { ); } + #[test] + fn outbound_tcp_diag_rejects_listeners_without_rejecting_udp() { + let mut buf = vec![0u8; NLMSG_HDR_LEN + INET_DIAG_MSG_LEN]; + let len = buf.len() as u32; + buf[0..4].copy_from_slice(&len.to_ne_bytes()); + buf[4..6].copy_from_slice(&SOCK_DIAG_BY_FAMILY.to_ne_bytes()); + buf[NLMSG_HDR_LEN + 68..NLMSG_HDR_LEN + 72].copy_from_slice(&31337u32.to_ne_bytes()); + for state in [0x01, 0x02, 0x0A] { + buf[NLMSG_HDR_LEN + 1] = state; + assert_eq!( + parse_response(&buf, libc::IPPROTO_TCP as u8).map(|i| i.inode), + (state != 0x0A).then_some(31337) + ); + } + buf[NLMSG_HDR_LEN + 1] = 0x07; + assert_eq!( + parse_response(&buf, libc::IPPROTO_UDP as u8).map(|i| i.inode), + Some(31337) + ); + } + #[test] fn error_reply_is_none() { // NLMSG_ERROR (type 2) reply, as the kernel sends for a miss. @@ -459,17 +485,19 @@ mod tests { buf[0..4].copy_from_slice(&len.to_ne_bytes()); buf[4..6].copy_from_slice(&2u16.to_ne_bytes()); buf[NLMSG_HDR_LEN..].copy_from_slice(&(-2i32).to_ne_bytes()); // -ENOENT - assert_eq!(parse_response(&buf), None); + assert_eq!(parse_response(&buf, libc::IPPROTO_TCP as u8), None); } #[test] fn truncated_reply_is_none() { - assert_eq!(parse_response(&[0u8; 8]), None); + assert_eq!(parse_response(&[0u8; 8], libc::IPPROTO_TCP as u8), None); let mut buf = vec![0u8; NLMSG_HDR_LEN + 8]; let len = buf.len() as u32; buf[0..4].copy_from_slice(&len.to_ne_bytes()); buf[4..6].copy_from_slice(&SOCK_DIAG_BY_FAMILY.to_ne_bytes()); - assert_eq!(parse_response(&buf), None); + assert_eq!(parse_response(&buf, libc::IPPROTO_TCP as u8), None); + buf[0..4].copy_from_slice(&8u32.to_ne_bytes()); + assert_eq!(parse_response(&buf, libc::IPPROTO_TCP as u8), None); } #[test] diff --git a/crates/cfc-ebpf-common/src/lib.rs b/crates/cfc-ebpf-common/src/lib.rs index d27a16c..e100a49 100644 --- a/crates/cfc-ebpf-common/src/lib.rs +++ b/crates/cfc-ebpf-common/src/lib.rs @@ -113,7 +113,9 @@ const _: () = { /// has seen `exec`, so a default deny here would blackhole every process that /// started before the daemon did - including the ones that bring the network /// up. The fail-closed guarantee stays where it already was, in the nftables -/// ruleset (`ct state new queue num 0`, no `bypass`). +/// ruleset: fail-closed for everything except new loopback flows, which are +/// allowed while no daemon listens (the final `ct state new queue num 0` has +/// no `bypass`). /// /// The point of this layer is the *opposite* direction: a deny written here /// keeps being enforced after the daemon is gone, because the link is pinned. diff --git a/crates/cfc-proto/proto/cfc.proto b/crates/cfc-proto/proto/cfc.proto index e71d75c..f69ad7e 100644 --- a/crates/cfc-proto/proto/cfc.proto +++ b/crates/cfc-proto/proto/cfc.proto @@ -251,17 +251,10 @@ message StatusResponse { // "pinned" and "inherited" are the two that survive the daemon. string enforcement = 15; - // Whether process-wide allows are skipping the queue: "live", "off: ", - // or empty when the daemon predates this field or startup has not answered - // yet. - // - // It carries its reason because the path has several ways to be inert - // with nothing else changing - the switch left off, an attach inherited - // from a build without it, a kernel whose verifier lacks bpf_setsockopt on - // sock_addr, exit tracking without group_dead, an nftables set the snippet - // does not declare - and an allow that quietly takes the slow path looks - // exactly like one that is working, only later. - string fast_allow = 16; + // Field 16 was `fast_allow`, the state of the removed Fast Allow path. + // Never reuse the number or the name. + reserved 16; + reserved "fast_allow"; } message SetPausedRequest { diff --git a/crates/cfc-proto/src/lib.rs b/crates/cfc-proto/src/lib.rs index ba073e0..a7b18a7 100644 --- a/crates/cfc-proto/src/lib.rs +++ b/crates/cfc-proto/src/lib.rs @@ -3,6 +3,10 @@ //! Generated from `proto/cfc.proto`. Speaks daemon <-> UI/CLI over a Unix //! domain socket (typically `/run/colony-firewall/cfc.sock`). +// Generated code. Clippy 1.99's double_must_use fires inside the +// #[async_trait] that tonic emits for the server trait; nothing here can +// change that. unknown_lints keeps older clippy versions quiet about the name. +#[allow(unknown_lints, clippy::double_must_use)] pub mod v1 { tonic::include_proto!("cfc.v1"); } diff --git a/crates/xtask/src/main.rs b/crates/xtask/src/main.rs index c69df89..404a94d 100644 --- a/crates/xtask/src/main.rs +++ b/crates/xtask/src/main.rs @@ -106,8 +106,6 @@ const REQUIRED_SYMBOLS: &[&str] = &[ // sock_addr programs; the loader tries the cookie ones first "cfc_connect4_basic", "cfc_connect6_basic", - "cfc_sendmsg4", - "cfc_sendmsg6", // maps "EXEC_EVENTS", "EXIT_EVENTS", @@ -122,8 +120,8 @@ const REQUIRED_SYMBOLS: &[&str] = &[ // "is it worth hashing?" guard "EXE_RULES", "EXE_RULES_ON", - // the fast path: the grant map, the deadline the daemon's heartbeat - // refreshes, the mark to set, and the ring the grants are reported on + // legacy Fast Allow maps: pinned by name so the daemon can disarm what an + // older release armed; gone with the kernel side at the next ABI bump "FAST_ALLOW", "FAST_ALLOW_UNTIL", "FAST_ALLOW_MARK", diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index d1817d0..4da8559 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -50,7 +50,8 @@ headless machine gets a say. ``` kernel (nftables OUTPUT hook) | - | loopback / established,related / daemon refusal packets accepted + | established,related / daemon refusal packets accepted + | oifname lo ct state new queue num 0 bypass (accepted if no daemon) | ct state new queue num 0 | all other traffic dropped v @@ -115,7 +116,7 @@ lands just after the worker committed to a fresh wait. `scripts/vm-bench` attributes it - 4.90 ms of 5.67 at 300 flows, 5.24 ms of 7.61 at 3000, by building the same daemon with the constant at 200 us and measuring both in one boot. These are historical measurements, not a current performance guarantee. -Fast Allow is disabled, so allowed flows also pay the queue round trip. +Fast Allow was removed, so allowed flows also pay the queue round trip. **Prompt deduplication** requires the same UID, executable path, image digest, destination IP, destination port and protocol. Source address and port are @@ -392,19 +393,23 @@ inert as one built with `--no-default-features`. | `tracepoint/sched/sched_process_exit` | `sched:sched_process_exit` | evicts only on confirmed thread-group death | | `cgroup_skb/ingress` | cgroup v2 root | copies received DNS response payloads for diagnostics, never policy identity | | `cgroup/connect4`, `cgroup/connect6` | cgroup v2 root, link **pinned** | refuse `connect()` for pids the daemon has denied outright, before a packet exists | -| `cgroup/sendmsg4`, `cgroup/sendmsg6` | cgroup v2 root, link pinned | legacy mark-clearing support; Fast Allow stays disabled | **In-kernel denials.** The connect hooks refuse an executable denied process-wide with `EPERM`. Pinned denials outlive the daemon. Conditional rules, prompts and Allow decisions remain on the normal NFQUEUE path. -**Fast Allow is disabled in every runtime configuration.** A socket mark cannot +**Fast Allow was removed.** It marked the sockets of a process a lasting Allow +covered so that nftables accepted them ahead of the queue. A socket mark cannot prove the current sender's identity, and lifecycle checks do not repair that -property. `fast_allow = true` produces a warning and no grants or heartbeat. -The nft snippet has no mark-set accept rule. Startup flushes legacy accepted -marks, and package upgrades reload active nft units with one atomic transaction -to remove old acceptance rules. A failed cleanup emits an error and requires -operator action before filtering can be relied upon. +property, so it opened bypasses; it was disabled in 0.7.0 and its userspace +side is gone. The `[ebpf] fast_allow` keys still parse and only log a warning. +The kernel object still carries the Fast Allow maps until an ABI bump, so +startup flushes the legacy nft set once, disarms the pinned maps (unarmed mark, +zero deadline, no grants) and removes the old `sendmsg4`/`sendmsg6` link pins, +which detaches those hooks. The nft snippet has no mark-set accept rule, and +package upgrades reload active nft units with one atomic transaction. A failed +flush emits an error and requires operator action before filtering can be +relied upon. **Compatibility exit handling.** When `sched_process_exit` exposes `group_dead`, the kernel evicts only on confirmed process death. Without that field, it diff --git a/docs/HARDENING.md b/docs/HARDENING.md index ed53008..5ed20d0 100644 --- a/docs/HARDENING.md +++ b/docs/HARDENING.md @@ -11,7 +11,7 @@ desktop, not what's theoretically pure. 2. Click through prompts for a week. Save persistent rules as you go. 3. Run `cfc rules bootstrap-defaults` to install common system rules. 4. Once the prompt rate drops to maybe 1-2 a day, switch to - `profile = "strict"` for fail-closed behavior. + `profile = "strict"` if you prefer a shorter prompt timeout. 5. Audit `cfc rules list` monthly. Remove rules for apps you no longer use, and check `cfc log --since 30d` for destinations you did not expect. @@ -23,14 +23,14 @@ running" throughout - it subscribes the same way the GUI does. | Profile | No UI | Timeout | Window | Use when | |----------|----------|----------|--------|------------------------------------------------| -| relaxed | Allow | Deny | 60s | Headless servers / can't always be at the UI | -| balanced | Allow | Deny | 30s | Daily-driver workstations (default) | -| strict | Deny | Deny | 15s | Lockdown posture, UI always present | +| relaxed | Deny | Deny | 60s | Longer time to answer prompts | +| balanced | Deny | Deny | 30s | Daily-driver workstations (default) | +| strict | Deny | Deny | 15s | Shorter time to answer prompts | -**No profile ever permits a connection by itself.** Not on timeout, not +**No profile ever permits a remote connection by itself.** Not on timeout, not when nothing is subscribed. The presets differ only in how long a prompt -waits for an answer. Only a stored rule, or a person answering, allows -traffic. +waits for an answer. Under these presets, a stored rule or a prompt answer +permits remote traffic. Unmatched local IPC is allowed without prompting. A timeout means the question *was* put to you and went unanswered; if that granted access, the cheapest attack would be to connect while @@ -58,8 +58,8 @@ before the daemon and `network-pre.target`. Enabled enforcement is required by NetworkManager and systemd-networkd, so an nft load failure blocks their startup. Initial daemon failure leaves the table loaded and drops new flows. This does not cover initramfs networking, already configured interfaces, or -other network managers. Once loaded, strict -filtering denies unmatched flows, so DHCP, DNS and NTP need standing rules or +other network managers. Once loaded, strict filtering denies unmatched remote +flows, so DHCP, DNS and NTP need standing rules or the machine cannot even get a lease. Network managers retrying DNS will look like total network failure. **Only flip to strict after you have rules for every always-on system service**. @@ -174,20 +174,37 @@ real path under `/usr/lib/...` or pin by SHA-256 (`scope.exe_sha256`). ## What this firewall does *not* protect against +Normal mode follows the desktop application firewall model of OpenSnitch and +Windows Firewall Control. It filters new tracked IP flows using socket +attribution. [Explicit application confinement](../README.md#explicit-application-confinement) +is a separate launch mode. + - **Anything from root**: `/usr/bin/colony-firewalld` itself is trusted, and so is any other root process. Use this firewall alongside, not instead of, traditional access controls. - **eBPF / unprivileged user namespaces**: a sufficiently privileged user can bypass NFQUEUE entirely with `unshare -rn` and a custom net namespace. -- **Local relays and DNS**: loopback is exempt. A denied application can use - an allowed local resolver or proxy; outbound traffic is attributed to that - service. Hostname rules and observed answers do not isolate DNS queries. +- **Local relays and DNS**: while the daemon runs, explicit rules apply to + new direct loopback flows and unmatched local IPC is allowed without + prompting. While no daemon listens on the queue, new loopback flows are + allowed unfiltered (`bypass` on the `lo` rule only): an explicit loopback + Deny or Reject rule is not enforced in that window, nothing records those + flows, and a loopback connection opened then keeps its authorization once + the daemon is back. An authorized local + resolver or proxy can relay remote traffic, which is attributed to that + service. CFC cannot establish the originating application's identity from + remote flows delegated through AF_UNIX or D-Bus brokers. Existing local + connections retain their authorization. Hostname rules and observed answers + do not isolate DNS queries. - **Inherited or passed sockets**: established/related traffic keeps its connection-wide authorization. An inherited or passed descriptor is not - reauthorized for each sending executable. -- **Packet-layer privileges**: applications with `CAP_NET_RAW` can use packet - sockets outside the shipped IP OUTPUT hooks. These rules do not provide - layer-2 containment. + reauthorized for each sending executable. Current descriptor ownership + and validated eBPF hints reduce false attribution; neither proves which + process sent a packet. +- **Raw and packet sockets**: applications with `CAP_NET_RAW` can use AF_PACKET + outside the shipped `inet OUTPUT` hook. Raw IP packets can coincide with + another socket's tuple even when TCP matching is strict. Tuple and inode + checks do not prove raw packet provenance or provide layer-2 containment. - **DNS-over-HTTPS embedded in browsers**: the firewall sees the outer HTTPS flow. Domain isolation requires an application-aware proxy or separate containment. - **Container traffic**: Docker / Podman / LXC route through their own @@ -345,7 +362,7 @@ to shrink what a code-execution bug could reach: | Directive | Why | |------------------------------------|---------------------------------| -| `CapabilityBoundingSet`, `AmbientCapabilities` | Seven capabilities, not full root: `CAP_NET_ADMIN` for NFQUEUE and for flushing legacy Fast Allow state, `CAP_NET_RAW` for Reject injection, `CAP_SYS_PTRACE` for reading other processes' `/proc`, `CAP_BPF` + `CAP_PERFMON` for the eBPF layer, `CAP_CHOWN` for the control socket's group, and `CAP_DAC_READ_SEARCH` for the `/proc/*/fd` walk attribution falls back to. The count and the list have to agree: this said seven and named five, and the two it left out are exactly the pair the SELinux policy was once missing - with the fail-closed ruleset, a daemon that cannot read `/proc` attributes nothing and the machine loses outbound traffic | +| `CapabilityBoundingSet`, `AmbientCapabilities` | Seven capabilities, not full root: `CAP_NET_ADMIN` for NFQUEUE, the nftables table probe and the one-shot flush of the legacy Fast Allow set, `CAP_NET_RAW` for Reject injection, `CAP_SYS_PTRACE` for reading other processes' `/proc`, `CAP_BPF` + `CAP_PERFMON` for the eBPF layer, `CAP_CHOWN` for the control socket's group, and `CAP_DAC_READ_SEARCH` for the `/proc/*/fd` walk attribution falls back to. The count and the list have to agree: this said seven and named five, and the two it left out are exactly the pair the SELinux policy was once missing - with the fail-closed ruleset, a daemon that cannot read `/proc` attributes nothing and the machine loses outbound traffic | | `NoNewPrivileges` | No regaining privileges via setuid binaries | | `SystemCallFilter=@system-service` | seccomp; the biggest blast-radius reduction available | | `SystemCallFilter=bpf perf_event_open` | The two syscalls the eBPF layer needs, named individually | @@ -361,8 +378,8 @@ to shrink what a code-execution bug could reach: **`ProtectProc=invisible` is deliberately absent.** It would hide other processes' `/proc` entries from the daemon, and that is precisely how process attribution works: `/proc/net/{tcp,udp}` gives a socket inode, -and the owning pid is found by walking `/proc/*/fd` for a matching -`socket:[inode]` link. Turning it on makes every connection resolve to an +and a current descriptor holder is found by walking `/proc/*/fd` for a +matching `socket:[inode]` link. Turning it on makes every connection resolve to an unknown process, which defeats the entire tool. Same reason `CAP_SYS_PTRACE` is in the bounding set. If you are hand-editing the unit, do not "harden" either of these. @@ -408,8 +425,9 @@ to `cgroupfs`. The other half of the security posture is the nftables side, not the daemon: whether the kernel drops or accepts new connections when nobody -is answering the queue. The shipped snippet is fail-closed, which is the -safer default and also the one that can lock you out of a remote box. +is answering the queue. The shipped snippet is fail-closed for everything +except new loopback flows, which are allowed while no daemon listens. That +is the safer default and also the one that can lock you out of a remote box. The full matrix - daemon up or down, table loaded or not, with and without `bypass` - is in [TROUBLESHOOTING.md](TROUBLESHOOTING.md#fail-open-vs-fail-closed-matrix). @@ -417,7 +435,8 @@ Read it before enabling enforcement on a machine you only reach over SSH. `[nfqueue] fail_open` must be `false`; `true` is rejected. Queue overflow must drop traffic instead of bypassing policy and durable refusal auditing. -The nftables `bypass` keyword governs missing listeners and is not shipped. +The nftables `bypass` keyword governs missing listeners; the shipped snippet +uses it only on the loopback rule (`oifname "lo"`). ## When something stops working diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 0ba6d03..6471335 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -3,9 +3,9 @@ Tracking the port from opensnitch (Go daemon + Python Qt UI) to Rust. Phases 0-3 are done and have since been through a hardening pass -(Phase 3.5). The eBPF backend, the system tray and the whitelist fast -path (opt-in, `[ebpf] fast_allow`; its latency win is still to be -measured, see `TODO.md` 1a) have since landed too. What is left is +(Phase 3.5). The eBPF backend and the system tray have since landed +too; the whitelist fast path landed and was later removed (see +`TODO.md` 1a). What is left is VirusTotal lookups, publishing to the AUR, and one end-to-end test that is still manual. @@ -165,16 +165,10 @@ kernel 7.1.8. 1,000,000-instruction complexity limit (see `crates/cfc-ebpf/README.md` for the full write-up) - [x] Whitelist fast path for already-allowed flows (`[ebpf] - fast_allow`). Not the shape first imagined: a cgroup *egress* - hook runs after NF_INET_LOCAL_OUT and cannot short-circuit - NFQUEUE, but the `connect()` hook runs before any packet exists. - A process a lasting Allow rule covers gets an entry in a kernel - map; its TCP connects are marked with SO_MARK at connect time - and `meta mark @fast_allow accept` takes them before the queue - rule. TCP only, grants evicted on exec and exit, the whole path - bounded by a heartbeat deadline so a dead daemon strips itself - out. Two degradations shorten the deadline instead of turning - the feature off; see `docs/ARCHITECTURE.md` + fast_allow`): removed. Allowed TCP connects were marked with + SO_MARK at connect time and accepted ahead of the queue rule, and + a socket mark does not attest which process sends, so it opened + bypasses; see `TODO.md` 1a ## Phase 5 - Polish diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index fd5093f..ff0f900 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -19,9 +19,10 @@ run `systemctl daemon-reload`, then `systemctl reenable colony-firewalld colony-firewall-nft` (and the inbound unit only if already enabled), and `systemctl reload colony-firewall-nft` (and the inbound unit if active) before relying on the new rules. Reenable installs the native network-manager -requirements on existing deployments. A startup error saying -previous Fast Allow state could not be disabled means old acceptance may still -exist; resolve that error and inspect the loaded table. The nft units load +requirements on existing deployments. A startup error saying the +legacy fast_allow nftables set could not be flushed means a mark left by an +older release may still be accepted; resolve that error and inspect the loaded +table. The nft units load before the daemon. Failed daemon initialization leaves filtering installed; a failed nft load blocks the daemon and the enabled NetworkManager or systemd-networkd requirements. This covers those managers' startup after @@ -36,11 +37,15 @@ this check. `CFC_INBOUND_FORCE=1` remains the explicit console override. ## Testing over SSH without locking yourself out -The shipped nftables snippet is **fail-closed**: `queue num 0` without the -`bypass` keyword means that if nothing is listening on NFQUEUE 0 (daemon -stopped, crashed, or not yet started), the kernel drops every *new* -outbound connection. Your established SSH session survives (`ct state new` -only matches new flows), but the moment it drops you cannot open a new one. +The shipped nftables snippet is **fail-closed for everything except new +loopback flows, which are allowed while no daemon listens**: the final +`queue num 0` without the `bypass` keyword means that if nothing is +listening on NFQUEUE 0 (daemon stopped, crashed, or not yet started), the +kernel drops every *new* non-loopback outbound connection. Only the rule +just above it, `oifname "lo" ct state new queue num 0 bypass`, lets new +loopback flows through in that state. Your established SSH session +survives (`ct state new` only matches new flows), but the moment it drops +you cannot open a new one. Three layers of protection, use all of them the first time: @@ -97,8 +102,8 @@ cfc status ``` If `systemctl` shows the unit dead while the nftables rule is loaded, you -are in the fail-closed state described above: packets are queued to NFQUEUE -0 and nobody answers. Start the daemon or delete the table. +are in the fail-closed state described above: non-loopback packets are +queued to NFQUEUE 0 and nobody answers. Start the daemon or delete the table. **Is the nftables table actually loaded?** @@ -119,7 +124,8 @@ If they differ, packets queue to a number nobody consumes - same lockout as a dead daemon. **The fail-open alternative.** If you would rather lose filtering than -lose the network when the daemon is down, add the `bypass` keyword: +lose the network when the daemon is down, add the `bypass` keyword to the +final queue rule too: ``` ct state new queue num 0 bypass @@ -236,27 +242,28 @@ them apart from a bad argument (2) or a missing rule (3). The snippet's `output` hook matches loopback traffic too. On systems using systemd-resolved, every DNS query goes to the stub resolver at -`127.0.0.53:53` - over loopback - so each lookup gets intercepted and can -prompt, time out, or (under `strict`) be denied. The symptom is DNS that -is slow, flaky, or dead while direct-by-IP connections work. - -Exempt loopback above the queue rule: +`127.0.0.53:53` - over loopback. The shipped ruleset queues new loopback +flows with their own rule, just above the final queue rule: ``` -table inet colony_firewall { - chain output { - type filter hook output priority 0; policy accept; - oifname lo accept - ct state new queue num 0 - } -} +oifname "lo" ct state new queue num 0 bypass +ct state new queue num 0 ``` -Loopback traffic never leaves the machine, so exempting it costs you no -outbound coverage. The ruleset installed by the companion -`colony-firewall-nft.service` unit includes this exemption; the caveat -applies mainly if you carry an older copy of the snippet in your own -`/etc/nftables.conf`. +While the daemon runs, it judges them like any other flow: explicit rules +apply, and unmatched local IPC (the stub resolver, CUPS, a local dev +server) is allowed without prompting. While nothing listens on the queue, +`bypass` makes the kernel accept them, so local IPC keeps working when the +daemon is down. That includes the stub resolver's socket, but only for names +it can answer from its cache or local records: its queries to the upstream +servers are new non-loopback flows, so resolving anything else still needs +the daemon. Every non-loopback new flow still meets the fail-closed rule. + +If you carry an older copy of the snippet in your own `/etc/nftables.conf`, +compare it with the shipped one: a copy without the loopback rule drops +every new loopback flow whenever the daemon is down, and one with an +explicit `oifname lo accept` skips the daemon for loopback entirely, so +loopback rules never apply. Note the daemon already exempts its *own* reverse-DNS lookups internally (they would otherwise deadlock the queue); the loopback rule is about @@ -266,10 +273,10 @@ everyone else's DNS. What happens to a **new outbound connection** in each state: -| State | Without `bypass` (shipped) | With `bypass` | +| State | Without `bypass` on the final rule (shipped) | With `bypass` | |------------------------------------|--------------------------------|--------------------------------| | Daemon up, nft rule loaded | Filtered: rules, then prompts, then profile fallback | Same | -| Daemon down, nft rule loaded | **Dropped. Total outbound lockout.** | Allowed, unfiltered (silent) | +| Daemon down, nft rule loaded | **Dropped. Outbound lockout** (new loopback flows still allowed) | Allowed, unfiltered (silent) | | Daemon up, nft rule *not* loaded | Allowed, unfiltered (silent - daemon sees nothing) | Same | | Daemon paused (`cfc pause`) | Rules still enforced; only *unmatched* flows pass instead of prompting. Auto-resumes | Same | diff --git a/packaging/rpm/colony-firewall-control.spec b/packaging/rpm/colony-firewall-control.spec index 51504b7..1adcfd8 100644 --- a/packaging/rpm/colony-firewall-control.spec +++ b/packaging/rpm/colony-firewall-control.spec @@ -59,10 +59,11 @@ Install one - `cargo xtask build-ebpf`, dropped at /usr/lib/colony-firewall/cfc-ebpf.o, in the directory this package creates for it - and the daemon adds process attribution, DNS display enrichment, and in-kernel connect(2) denial. Pinned denials survive a daemon crash. Fast Allow -is disabled; allowed connections continue through NFQUEUE. +was removed; allowed connections go through NFQUEUE. -The ruleset is fail-closed. If the daemon is not running, new outbound -connections are dropped rather than allowed. +The ruleset is fail-closed for everything except new loopback flows. If the +daemon is not running, new non-loopback outbound connections are dropped +rather than allowed; new loopback flows are allowed so local IPC keeps working. %package selinux Summary: SELinux policy module for %{name} @@ -79,7 +80,7 @@ SELinux policy module for Colony Firewall Control. Confines the daemon to what it actually needs: netlink_netfilter and raw sockets, bpf() and perf_event_open(), the bpffs pin directory, other domains' /proc entries for attribution, a read-only rpm query for package provenance, -and running nft(8) to clear Fast Allow state left by older installations. CAP_SYS_ADMIN is deliberately not granted; that it is +and running nft(8) to probe the table and flush the legacy Fast Allow set. CAP_SYS_ADMIN is deliberately not granted; that it is unnecessary is covered by a test rather than assumed. %prep diff --git a/packaging/selinux/README.md b/packaging/selinux/README.md index 1b01d1e..a7a1197 100644 --- a/packaging/selinux/README.md +++ b/packaging/selinux/README.md @@ -31,7 +31,7 @@ tells you which kind of trouble you are in. | group | denial costs | |---|---| -| netlink_netfilter, raw sockets | **everything.** The daemon exits before `READY=1`, and the ruleset is fail-closed, so the machine loses outbound network | +| netlink_netfilter, raw sockets | **everything.** The daemon exits before `READY=1`, and the ruleset is fail-closed (except new loopback flows), so the machine loses non-loopback outbound network | | unix socket under `/run` | the CLI, tray and GUI cannot reach the daemon; filtering continues, unattended | | `bpf`, `perf_event`, tracefs, cgroup | the ring-0 layer. Attribution falls back to `sock_diag` + `/proc`, hostnames to PTR lookups. Logged once, then filtering continues | | bpffs (`/sys/fs/bpf`) | in-kernel denials no longer survive the daemon being killed. Silent apart from `enforcement=process` in the startup line | diff --git a/packaging/selinux/TESTING.md b/packaging/selinux/TESTING.md index fff726f..cf51101 100644 --- a/packaging/selinux/TESTING.md +++ b/packaging/selinux/TESTING.md @@ -11,8 +11,9 @@ protocol for whoever has such a host. Run it once, report what you see, and ## What you need - A Rocky 9 or Fedora VM with SELinux enforcing (`getenforce` says - `Enforcing`). A VM, not your workstation: the ruleset is fail-closed, and a - policy gap in the wrong group takes the machine's outbound network down. + `Enforcing`). A VM, not your workstation: the ruleset is fail-closed + (except new loopback flows), and a policy gap in the wrong group takes the + machine's outbound network down. For the same reason, have **console access**, not just SSH. - The audit tooling: `dnf install audit policycoreutils-python-utils`. `semanage` and `audit2allow` live in the second package, and on a minimal @@ -37,8 +38,9 @@ AVC in the audit log **without being enforced** - observed, not suffered. Why that ordering matters here more than for most policies, in the module's own words: a denied `netlink_netfilter` socket is not a degraded feature, it is a daemon that exits before `READY=1` - and because the nftables ruleset is -fail-closed (`ct state new queue num 0`, no `bypass`), a daemon that does not -come up takes the machine's outbound network with it. Running the first pass +fail-closed for everything except new loopback flows, which are allowed while +no daemon listens, a daemon that does not come up takes the machine's +non-loopback outbound network with it. Running the first pass permissive converts that outage into a log line. Dontaudit rules hide denials, and this module carries some @@ -109,7 +111,6 @@ chance to be needed. | bpf/perf ring 0 | **degraded by design in the RPM**: no eBPF object ships (see the spec's `%build` comment), so the journal says `ring0=unavailable degrade=object_missing` once at startup and the bpf/perf/bpffs/tracefs rules are never reached. That log line *is* the expected result. To exercise the group for real: build the object (`cargo xtask build-ebpf`, pinned nightly + bpf-linker), drop it at `/usr/lib/colony-firewall/cfc-ebpf.o`, restart, and expect `ring0=active` | with the object installed: `degrade=not_permitted` where `object_missing` was, and AVCs on `bpf`, `perf_event`, `tracefs_t`/`debugfs_t` or `bpf_t` | | /proc attribution walk | `curl` from a second user account; the prompt must name curl's real path and pid | every prompt says `exe= pid=0`; AVCs from `domain_read_all_domains_state` targets. Enforcing, this is the outage mode: no exe rule can ever match | | rpm provenance | automatic: one `rpm -qa` at startup and after any `dnf install`. Install any small package, wait ~2 minutes, then check a prompt or `cfc log` shows package names | everything reports `Unpackaged` plus one provenance warning in the journal; AVC on `rpm_exec_t` or `rpm_var_lib_t` | -| nft, the fast-allow set | needs ring 0 up (the object installed as in the bpf/perf row) and `[ebpf] fast_allow = true`, then both units running. Fast-allow armed: `cfc status` shows fast_allow live, `sudo nft list set inet colony_firewall fast_allow` shows one element. Then set `fast_allow = false` and `systemctl restart colony-firewalld`: the set is empty while the table is still loaded, which is the unconditional start-up flush. (Do **not** test this by stopping the daemon - `colony-firewall-nft.service` is `PartOf=` it and tears the whole table down first, so `list set` answers "No such file or directory" and tells you nothing about the flush.) | `cfc status` shows fast_allow off with an nft error as the reason, and the set stays empty; AVC on `iptables_exec_t` (execute) - the `netlink_netfilter_socket` nft needs is the filtering group's, already exercised by the first row | | control socket, unconfined client | `cfc status` and `cfc rules list` as a normal logged-in user in the `colony-firewall` group (not root, not sudo) | connection refused/denied; AVC with the client's domain (`unconfined_t`) and `colony_firewall_runtime_t` | | sqlite WAL in /var/lib | answer any prompt with a persistent choice (**a**, then `3`=always), then `ls /var/lib/colony-firewall/` - `rules.db-wal` and `rules.db-shm` must exist while the daemon runs | the `map` denial is the quiet one: no error anywhere, just journal-mode SQLite and a 2.5x write regression. An AVC with class `file` permission `map` on `colony_firewall_var_lib_t` is the tell | diff --git a/packaging/selinux/colony_firewall.te b/packaging/selinux/colony_firewall.te index 672db78..081db82 100644 --- a/packaging/selinux/colony_firewall.te +++ b/packaging/selinux/colony_firewall.te @@ -21,8 +21,8 @@ policy_module(colony_firewall, 0.2.0) # resolve struct offsets without CO-RE; # * walks other processes' /proc entries to attribute a socket to a program; # * runs rpm(8) once per package-database generation, for provenance; -# * runs nft(8) at start and at stop, to put its fast-allow mark into one -# nftables set and take it out again; +# * runs nft(8) once at start, to flush a legacy Fast Allow set, and once a +# minute, to ask whether the filtering table is loaded; # * serves a unix socket under /run/colony-firewall to the CLI, tray and GUI. # # Every one of those is a separate way to be denied, and the failure modes @@ -101,8 +101,9 @@ allow colony_firewalld_t self:unix_dgram_socket create_socket_perms; # # This is the group that must not fail. A denial here is not a degraded # feature, it is a daemon that exits before READY=1 - and because the nftables -# ruleset is fail-closed (`ct state new queue num 0`, no `bypass`), a daemon -# that does not come up takes the machine's outbound network with it. +# ruleset is fail-closed for everything except new loopback flows, which are +# allowed while no daemon listens, a daemon that does not come up takes the +# machine's non-loopback outbound network with it. # ######################################## @@ -190,28 +191,17 @@ optional_policy(` # check on the same file rather than the only one. libs_read_lib_files(colony_firewalld_t) -# The fast-allow path. The connect hooks mark the sockets of a process the -# daemon has ruled allowed process-wide, and the shipped snippet accepts that -# mark ahead of the queue - but only for values in a set the snippet declares -# empty. The daemon runs nft(8) to put its per-start random value in and to -# flush the set again, executed in this domain like rpm below rather than -# transitioning to iptables_t, which may rewrite the whole ruleset. nft then -# opens a netlink_netfilter socket, already allowed in the filtering group -# above: it is the socket class NFQUEUE itself lives on. -# -# Five subcommands, not the two an earlier version of this comment claimed: -# `flush set` and `add element` do the work, `list set` and `list table` are -# the probe that tells a missing set from a missing table, and `get element` -# is the periodic check that the mark is still accepted after a ruleset -# reload. Nor is it twice in a daemon's life: the nft unit starts *after* the -# daemon, so arming is a retry on every heartbeat - ten seconds, or two on a -# kernel that cannot pin the exec/exit tracepoint links - until the table -# appears, and once armed the check runs every sixty. On a host where this is -# denied that is an AVC every heartbeat, for as long as the daemon runs with -# fast_allow = true - loud on purpose, but worth knowing before reading a log. -# -# A denial costs the fast path and nothing else. The daemon reports it as off -# with the reason in `cfc status`, and every connection keeps taking the queue. +# nft(8), for two things. Once at start, `flush set` empties the legacy +# fast_allow set: Fast Allow was removed, and a 0.4-0.6 daemon that crashed +# while armed can have left its mark accepted there. Once a minute, `list +# table` tells `cfc status` whether the filtering table is loaded. Executed in +# this domain like rpm below rather than transitioning to iptables_t, which may +# rewrite the whole ruleset. nft then opens a netlink_netfilter socket, already +# allowed in the filtering group above: it is the socket class NFQUEUE itself +# lives on. +# +# A denial costs the flush (an error in the journal at start) and the probe +# (`enforcing` keeps its previous answer); filtering itself is unaffected. # # Fedora labels /usr/bin/nft iptables_exec_t (RHEL: /usr/sbin/nft); the same # type covers both spellings, and the daemon tries both paths. diff --git a/pkg/README.md b/pkg/README.md index 94c0936..cbc8b65 100644 --- a/pkg/README.md +++ b/pkg/README.md @@ -27,7 +27,7 @@ Key design points: - **`colony-firewall-nft.service`** makes enforcement persistent (`nft -f` the snippet on start, `nft delete table inet colony_firewall` on explicit nft-unit stop). The table survives daemon restarts and stops, - so new flows fail closed while its queue listener is absent. Upgrades reload + so new non-loopback flows fail closed while its queue listener is absent. Upgrades reload active nft units atomically and leave inactive inbound filtering opt-in. The daemon requires this unit before initialization. Enabling either nft unit creates native `Requires` links from NetworkManager and diff --git a/pkg/colony-firewall-control.install b/pkg/colony-firewall-control.install index b69ae22..ffc1231 100644 --- a/pkg/colony-firewall-control.install +++ b/pkg/colony-firewall-control.install @@ -7,7 +7,8 @@ post_install() { 1. Enable the daemon and the persistent nftables rules: systemctl enable --now colony-firewalld colony-firewall-nft (colony-firewall-nft loads 'table inet colony_firewall'; it is - fail-closed while loaded and survives daemon restarts/stops. + fail-closed while loaded, except new loopback flows, and survives + daemon restarts/stops. Stop colony-firewall-nft explicitly to remove filtering.) 2. Let your desktop user talk to the daemon socket: @@ -51,7 +52,7 @@ EOF pre_remove() { # Stop enforcement BEFORE the binaries and the snippet disappear, so # the fail-closed NFQUEUE table can never outlive the daemon and - # blackhole all new outbound traffic. + # blackhole all new non-loopback outbound traffic. # # The inbound pair belongs here too, and used not to be. Removing the # package on a host where the inbound chain had been enabled left diff --git a/scripts/armed-e2e.sh b/scripts/armed-e2e.sh new file mode 100755 index 0000000..aca44da --- /dev/null +++ b/scripts/armed-e2e.sh @@ -0,0 +1,205 @@ +#!/usr/bin/env bash +# Armed end-to-end test: real NFQUEUE verdicts, not --dry-run. +# +# Everything filtered lives in a throwaway network namespace, so the host's +# own traffic (the CI runner's connection to GitHub, an SSH session) never +# meets the fail-closed table. Two namespaces joined by a veth pair: +# FW the shipped nftables snippet, colony-firewalld and curl +# SRV three HTTP servers, no filtering +# Needs sudo (passwordless on GitHub runners). Never runs nft outside FW. + +set -euo pipefail + +ROOT="$(cd "$(dirname "$0")/.." && pwd)" +DAEMON="${ROOT}/target/debug/colony-firewalld" +CFC="${ROOT}/target/debug/cfc" +FW="cfc-e2e-fw-$$" +SRV="cfc-e2e-srv-$$" +SRV_IP=10.200.0.2 +ALLOW_PORT=8080 # allow rule +DENY_PORT=8081 # deny rule +UNMATCHED_PORT=8082 # no rule: balanced profile, nobody subscribed -> Deny +LO_PORT=8083 # 127.0.0.1 inside FW, last step only +W="$(mktemp -d "${RUNNER_TEMP:-/tmp}/cfc-e2e.XXXXXX")" +SOCK="${W}/cfc.sock" + +fail() { echo "FAIL: $*" >&2; exit 1; } +say() { printf '\n=== %s ===\n' "$*"; } +in_fw() { sudo ip netns exec "${FW}" "$@"; } +in_srv() { sudo ip netns exec "${SRV}" "$@"; } +cfc() { sudo "${CFC}" --socket "${SOCK}" "$@"; } +table_loaded() { in_fw nft list table inet colony_firewall >/dev/null; } +# Kernel truth, not a log line: is anything bound to NFQUEUE 0 in FW? +queue_bound() { + in_fw awk '$1 == 0 { b = 1 } END { exit !b }' \ + /proc/net/netfilter/nfnetlink_queue 2>/dev/null +} + +# curl from inside FW as the unprivileged runner user. Prints the HTTP code +# and returns curl's exit status. +probe() { + in_fw setpriv --reuid="$(id -u)" --regid="$(id -g)" --clear-groups \ + curl -sS --noproxy '*' -o /dev/null -w '%{http_code}' \ + --connect-timeout 3 --max-time 6 "http://${SRV_IP}:$1/" +} +expect_200() { + local code + code="$(probe "$1")" || fail "port $1: curl failed, expected HTTP 200" + [[ "${code}" == 200 ]] || fail "port $1: got HTTP ${code}, expected 200" +} +# 28 = connect timeout: the SYN vanished. 7 (refused) would mean an RST came +# back, i.e. something answered instead of dropping. +expect_drop() { + local rc=0 + probe "$1" >/dev/null 2>&1 || rc=$? + [[ "${rc}" -eq 28 ]] || fail "port $1: curl exit ${rc}, expected 28 (silently dropped)" +} + +start_daemon() { + # The inner sh writes its own PID and then execs the daemon, so the file + # names the daemon itself (pkill -x cannot: comm is cut to 15 characters). + # shellcheck disable=SC2016 # $$, $0 and $@ belong to the inner sh + in_fw sh -c 'echo $$ >"$0"; exec "$@"' "${W}/daemon.pid" \ + "${DAEMON}" --debug --config "${W}/daemon.toml" --socket "${SOCK}" \ + >"${W}/$1.log" 2>&1 & + DAEMON_JOB=$! + for _ in $(seq 1 150); do + queue_bound && cfc status >/dev/null 2>&1 && return 0 + sleep 0.2 + done + fail "daemon did not bind NFQUEUE 0 and its socket within 30s" +} +stop_daemon() { + local pid + pid="$(sudo cat "${W}/daemon.pid")" + sudo kill -"$1" "${pid}" + for _ in $(seq 1 100); do + sudo kill -0 "${pid}" 2>/dev/null || break + sleep 0.2 + done + if sudo kill -0 "${pid}" 2>/dev/null; then fail "daemon survived SIG$1 for 20s"; fi + wait "${DAEMON_JOB}" 2>/dev/null || true + sudo rm -f "${W}/daemon.pid" +} + +cleanup() { + local rc=$? + if sudo test -s "${W}/daemon.pid"; then + sudo kill -KILL "$(sudo cat "${W}/daemon.pid")" 2>/dev/null || true + fi + for ns in "${SRV}" "${FW}"; do + sudo ip netns pids "${ns}" 2>/dev/null | xargs -r sudo kill 2>/dev/null || true + done + sudo ip netns del "${FW}" 2>/dev/null || true + sudo ip netns del "${SRV}" 2>/dev/null || true + if [[ "${rc}" -ne 0 ]]; then + for f in "${W}"/*.log; do + [[ -e "${f}" ]] || continue + echo "--- ${f} (last 200 lines)" + tail -n 200 "${f}" + done + fi + sudo rm -rf "${W}" +} +trap cleanup EXIT + +sudo -v +command -v jq >/dev/null || fail "jq is required" + +say "Building colony-firewalld (no eBPF) and cfc" +cargo build --locked -p cfc-daemon -p cfc-cli --no-default-features + +say "Namespaces, veth pair, HTTP servers" +sudo modprobe -a nfnetlink_queue nft_queue +sudo ip netns add "${FW}" +sudo ip netns add "${SRV}" +sudo ip link add fw0 netns "${FW}" type veth peer name srv0 netns "${SRV}" +in_fw ip addr add 10.200.0.1/24 dev fw0 +in_fw ip link set fw0 up +in_srv ip addr add "${SRV_IP}/24" dev srv0 +in_srv ip link set srv0 up +# FW's lo stays down on purpose while a daemon runs: no loopback flows (the +# daemon's own reverse DNS to a 127.0.0.53 stub fails fast instead of being +# queued). The last step brings it up, after the final daemon is gone. +mkdir "${W}/www" +for p in "${ALLOW_PORT}" "${DENY_PORT}" "${UNMATCHED_PORT}"; do + in_srv python3 -m http.server "${p}" --bind "${SRV_IP}" \ + --directory "${W}/www" >"${W}/http-${p}.log" 2>&1 & +done + +say "Control: every port answers before the firewall exists" +for p in "${ALLOW_PORT}" "${DENY_PORT}" "${UNMATCHED_PORT}"; do + for _ in $(seq 1 50); do + [[ "$(probe "${p}" 2>/dev/null)" == 200 ]] && break + sleep 0.2 + done + expect_200 "${p}" +done + +say "Load the shipped snippet in FW (as README: nft -f from a checkout)" +in_fw nft -f "${ROOT}/systemd/nftables-snippet.conf" +table_loaded || fail "table inet colony_firewall not loaded in ${FW}" +queue_bound && fail "something is already bound to NFQUEUE 0" + +say "Fail-closed before the daemon ever started" +expect_drop "${ALLOW_PORT}" + +cat >"${W}/daemon.toml" </dev/null \ + || fail "no deny event on port ${DENY_PORT} attributed to rule ${DENY_ID}" +echo "${DENIES}" | jq -e --argjson p "${UNMATCHED_PORT}" \ + 'any(.[]; .dst_port == $p and .rule_id == null)' >/dev/null \ + || fail "no default-deny event on port ${UNMATCHED_PORT}" + +say "SIGTERM: clean stop keeps the table, new flows drop" +stop_daemon TERM +table_loaded || fail "a clean daemon stop removed the table" +queue_bound && fail "NFQUEUE 0 still bound after the daemon exited" +expect_drop "${ALLOW_PORT}" + +say "Restart: rules persisted" +start_daemon daemon-2 +expect_200 "${ALLOW_PORT}" +expect_drop "${DENY_PORT}" + +say "SIGKILL: crash keeps the table, new flows drop" +stop_daemon KILL +table_loaded || fail "table gone after SIGKILL" +queue_bound && fail "NFQUEUE 0 still bound after SIGKILL" +expect_drop "${ALLOW_PORT}" + +say "No daemon: a new loopback flow still passes (bypass on lo only)" +in_fw ip link set lo up +in_fw python3 -m http.server "${LO_PORT}" --bind 127.0.0.1 \ + --directory "${W}/www" >"${W}/http-lo.log" 2>&1 & +for _ in $(seq 1 50); do + [[ -n "$(in_fw ss -Hltn "sport = :${LO_PORT}")" ]] && break + sleep 0.2 +done +in_fw curl -sS --noproxy '*' -o /dev/null --connect-timeout 3 --max-time 6 \ + "http://127.0.0.1:${LO_PORT}/" \ + || fail "new loopback flow failed with no daemon; the lo bypass rule should accept it" + +say "Armed e2e passed" diff --git a/scripts/bench-latency.sh b/scripts/bench-latency.sh index a3a2d15..4e8b20f 100755 --- a/scripts/bench-latency.sh +++ b/scripts/bench-latency.sh @@ -10,9 +10,8 @@ # # Two directions, because they do not take the same path: # out host -> namespace. Every SYN leaves through the host's output chain, -# where `inet colony_firewall` queues `ct state new` to the daemon - -# unless the client is fast-allowed, in which case its mark takes it -# past the queue. This is the direction the fast path exists for. +# where `inet colony_firewall` queues `ct state new` to the daemon. +# This is the direction that meets the queue. # in namespace -> host. The SYN arrives on the host's input path, which # `inet colony_firewall_inbound` filters only where that opt-in unit # is loaded. With it absent, this direction never meets a queue - but @@ -27,10 +26,9 @@ # from 10.199.0.0/24 there, or read `in` as unavailable on that host. # # The script never touches nftables, the daemon or its rules. It measures the -# machine as it finds it, prints what `cfc status` says the fast path is, and -# leaves the comparison to whoever runs it more than once: with the client -# covered by a lasting Allow rule (fast path), by a flow-scoped one (queue), -# and with the table absent (nothing). The client CFC attributes is python3, +# machine as it finds it, and leaves the comparison to whoever runs it more +# than once: with the client covered by an Allow rule (queue) and with the +# table absent (nothing). The client CFC attributes is python3, # so the rule to write is for python3's resolved path (`readlink -f # "$(command -v python3)"`); the first connect of a run is the one that prompts. # @@ -44,11 +42,11 @@ # SQLite bench, and the next one that writes should remember it. # # Needs root - it creates a namespace and a veth pair - plus iproute2 and -# python3. A VM is the right place: the point of measuring is to arm the fast -# path, and arming a firewall on a development host has consequences. +# python3. A VM is the right place: the point of measuring is to arm the +# firewall, and arming one on a development host has consequences. # # sudo scripts/bench-latency.sh both directions, 200 connects -# sudo scripts/bench-latency.sh -n 1000 -d out -l "fast path live" +# sudo scripts/bench-latency.sh -n 1000 -d out -l "queue armed" # sudo scripts/bench-latency.sh --json >> runs.jsonl one JSON object per direction set -euo pipefail @@ -245,19 +243,6 @@ wait_listening "" "$PORT" # each probe is allowed to fail: the bench is also how one measures a machine # with no CFC on it at all. KERNEL="$(uname -r)" -FAST_ALLOW="cfc not installed" -if command -v cfc >/dev/null 2>&1; then - # cfc exits non-zero when the daemon is down; under pipefail that failed - # the whole pipeline after python had already printed, and the `|| echo` - # that used to follow printed the same words a second time. - FAST_ALLOW="$( (cfc status --json 2>/dev/null || true) | python3 -c ' -import json, sys -try: - print(json.load(sys.stdin).get("fast_allow", "not reported")) -except Exception: - print("daemon not reachable") -')" -fi TABLES="nft not installed" if command -v nft >/dev/null 2>&1; then # `|| true` on the whole pipeline: a machine with no colony table makes @@ -269,7 +254,6 @@ if command -v nft >/dev/null 2>&1; then fi { echo "kernel: $KERNEL" - echo "fast-allow: $FAST_ALLOW" echo "nft tables: $TABLES" echo "link: $HOST_IF ($HOST_IP) <-> $NS:$NS_IF ($NS_IP), port $PORT" echo "per run: $COUNT connects after $WARMUP warm-up, ${TIMEOUT}s timeout each" @@ -281,13 +265,13 @@ run_direction() { # run_direction local dir="$1" ns="" target="$NS_IP" raw if [[ "$dir" == in ]]; then ns="$NS"; target="$HOST_IP"; fi raw="$(in_ns "$ns" python3 -c "$CLIENT" "$target" "$PORT" "$COUNT" "$TIMEOUT" "$WARMUP")" - python3 - "$raw" "$dir" "$LABEL" "$KERNEL" "$FAST_ALLOW" "$COUNT" "$JSON" "$PORT" <<'PY' + python3 - "$raw" "$dir" "$LABEL" "$KERNEL" "$COUNT" "$JSON" "$PORT" <<'PY' import json, sys r = json.loads(sys.argv[1]) -port_num = int(sys.argv[8]) +port_num = int(sys.argv[7]) r.update(direction=sys.argv[2], label=sys.argv[3], kernel=sys.argv[4], - fast_allow=sys.argv[5], connects=int(sys.argv[6])) -if sys.argv[7] == "1": + connects=int(sys.argv[5])) +if sys.argv[6] == "1": print(json.dumps(r)) sys.exit(0) f = r["failed"] diff --git a/scripts/vm-bench/README.md b/scripts/vm-bench/README.md index b5436ce..289d241 100644 --- a/scripts/vm-bench/README.md +++ b/scripts/vm-bench/README.md @@ -2,8 +2,7 @@ `scripts/bench-latency.sh` measures connect latency over a veth pair. It answers nothing on its own, because the interesting comparison needs CFC -*armed* - the queue rule loaded, the daemon deciding, the fast path granting - -and arming a fail-closed firewall on a development machine has consequences. +*armed* - the queue rule loaded, the daemon deciding - and arming a fail-closed firewall on a development machine has consequences. This directory boots a throwaway VM instead. It assembles an initramfs from the host's own kernel modules, `nftables`, `iproute2`, `python3` and the release @@ -28,22 +27,25 @@ isolates one cost. | state | what is running | what the difference against the previous one buys | |---|---|---| | `floor` | nothing: no daemon, no table | the veth link and `connect()` itself | -| `queue-N` | the daemon, the table, a lasting Allow, `fast_allow = false` | the NFQUEUE round trip, at N flows | +| `queue-N` | the daemon, the table, a lasting Allow | the NFQUEUE round trip, at N flows | | `poll200us-N` | the same, with a daemon built with a shorter `RECV_POLL_INTERVAL` | how much of that round trip is the worker's idle beat | -| `fast-N` | `fast_allow = true`, the client covered by a lasting Allow | the fast path against the queue | +| `lo-floor` | nothing; a UDP echo server on `127.0.0.1` inside the guest | the loopback round trip itself | +| `lo-queue` | the daemon and the table, same echo server | what the `oifname "lo" ... queue num 0 bypass` rule costs a new loopback flow | -Both directions run in every state and they answer different questions. `out` +The two `lo-*` states run after the sweep. The client opens a new socket per +round trip (1000 of them), so every round trip is a new conntrack flow, +and reports mean, p50, p90, p95, p99 and max under direction `lo`. + +Both directions run in every veth state and they answer different questions. `out` leaves through the host's output chain and meets the queue. `in` is generated inside the network namespace, whose own output chain carries no colony table, so it never meets a queue - but its client sits in the root cgroup and still runs the connect hooks, which makes `in` the cost of the eBPF layer alone. -Two things are recorded beside every measurement rather than assumed: -`cfc status`'s own account of the fast path, and `id_sequence` from -`/proc/net/netfilter/nfnetlink_queue` - one increment per packet the kernel -actually handed to userspace. A state calling itself `fast` whose queue saw one -packet per connect did not take the fast path, and no latency figure says that -on its own. +One thing is recorded beside every measurement rather than assumed: +`id_sequence` from `/proc/net/netfilter/nfnetlink_queue` - one increment per +packet the kernel actually handed to userspace. A state whose queue saw no +packets did not measure the queue, and no latency figure says that on its own. `ALT_DAEMON=/path/to/colony-firewalld` carries a second daemon into the same image, measured in the same boot under conditions that differ in nothing else. @@ -57,15 +59,13 @@ Run on 2026-09-06, Linux 7.2.2, KVM, four vCPUs, 3000 flows unless said. | state | 300 flows | 3000 flows | |---|---|---| | no firewall | 0.0158 ms | 0.0162 ms | -| fast path | 0.0268 ms | 0.0269 ms | | queue, 200 us idle beat | 0.7703 ms | 2.3646 ms | | queue, the shipped 5 ms beat | 5.6745 ms | 7.6083 ms | -Read across, and three things fall out. +That run also measured Fast Allow, since removed because a socket mark does +not prove which process sends (see `TODO.md` 1a). Read across, and two things +fall out. -- **The fast path saves 5.6 ms per new flow at 300 flows and 7.6 ms at 3000**, - and costs 0.011 ms over having no firewall at all. Its own cost does not grow - with load, because those flows never reach the daemon. - **A full `RECV_POLL_INTERVAL` is paid per queued flow, not half of one.** `crates/cfc-daemon/src/nfqueue.rs` predicts "up to one interval (mean: half that)", which is right for random arrivals and wrong for a client that diff --git a/scripts/vm-bench/init b/scripts/vm-bench/init index 66672f2..5953a74 100755 --- a/scripts/vm-bench/init +++ b/scripts/vm-bench/init @@ -13,9 +13,8 @@ mount -t sysfs sysfs /sys mount -t tmpfs tmpfs /tmp mount -t tmpfs tmpfs /run mount -t cgroup2 cgroup2 /sys/fs/cgroup -# bpffs, unlike the CI guests: with it the exec/exit links pin, which is what -# lets the fast path run with its full sixty-second deadline rather than the -# shortened one. The measurement should see the feature as a host sees it. +# bpffs, unlike the CI guests: with it the connect and exec/exit links pin, +# so the measurement sees the in-kernel layer as a host sees it. mount -t bpf bpf /sys/fs/bpf mount -t tracefs tracefs /sys/kernel/tracing 2>/dev/null diff --git a/scripts/vm-bench/plan.sh b/scripts/vm-bench/plan.sh index 58917e8..0bb82e4 100755 --- a/scripts/vm-bench/plan.sh +++ b/scripts/vm-bench/plan.sh @@ -2,10 +2,9 @@ # Why does a queued flow cost what it costs, and does that cost depend on load? # # The first full run said 17.8 ms per queued flow at 3000 flows and 5.5 ms at -# 40, with the two rounds 15.0 and 20.7 ms apart - and the state that does -# strictly MORE work (`armed`: the fast path live but the rule ineligible) came -# out faster than the one that does less. None of that is a per-packet -# constant. Two candidate explanations, and this run separates them: +# 40, with the two rounds 15.0 and 20.7 ms apart - and a state that did +# strictly MORE work came out faster than one that did less. None of that is a +# per-packet constant. Two candidate explanations, and this run separates them: # # 1. a fixed cost per flow, dominated by RECV_POLL_INTERVAL (5 ms), the beat # the NFQUEUE worker idles on. Testable by changing the constant: the same @@ -56,7 +55,6 @@ enabled = false [ebpf] enabled = "on" object_path = "/cfc-ebpf.o" -fast_allow = $1 EOF } @@ -84,31 +82,77 @@ stop_daemon() { probe_layer() { say "what the in-kernel layer comes up as here" - write_cfg true + write_cfg start_daemon /usr/bin/colony-firewalld info || return 1 nft -f "$SNIPPET"; write_rules cfc --socket "$SOCK" rules import --replace /tmp/rules.json >/dev/null 2>&1 sleep 4 - for k in ring0 enforcement degrade fast_path exec_tracking exit_tracking dns_capture ppid_from_btf; do + for k in ring0 enforcement degrade exec_tracking exit_tracking dns_capture ppid_from_btf; do v="$(grep -oE "$k=[A-Za-z_-]+" "$LOG" | tail -1)" [ -n "$v" ] && ctx "layer $v" done - ctx "layer $(cfc --socket "$SOCK" status --json | python3 -c 'import json,sys; d=json.load(sys.stdin); print("status_fast_allow=%s status_enforcing=%s" % (d["fast_allow"], d["enforcing"]))')" + ctx "layer $(cfc --socket "$SOCK" status --json | python3 -c 'import json,sys; d=json.load(sys.stdin); print("status_enforcing=%s" % d["enforcing"])')" stop_daemon } -measure() { # $1 label $2 n $3 mode(none|queue|fast) $4 binary +arm() { # $1 label $2 binary + write_cfg + start_daemon "$2" || { echo "FAIL $1"; return 1; } + nft -f "$SNIPPET" || { echo "FAIL $1 nft"; stop_daemon; return 1; } + write_rules + cfc --socket "$SOCK" rules import --replace /tmp/rules.json >/dev/null 2>&1 + sleep 4 +} + +# Loopback round trip: a UDP echo server on 127.0.0.1 and a client that opens +# one new socket (one new conntrack flow, so one queued packet when armed) per +# round trip. Armed, this is the snippet's `oifname "lo" ... bypass` rule. +measure_lo() { # $1 label $2 mode(none|queue) + local q0 q1 + say "state: $1 n=1000 mode=$2 loopback udp echo" + if [ "$2" != none ]; then arm "$1" /usr/bin/colony-firewalld || return 1; fi + q0="$(qseq)" + python3 - "$1" 1000 <<'ECHO' | while read -r line; do echo "RESULT $line"; done +import json, socket, statistics, sys, threading, time +label, n = sys.argv[1], int(sys.argv[2]) +srv = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) +srv.bind(("127.0.0.1", 0)) +def echo(): + while True: + data, peer = srv.recvfrom(64) + srv.sendto(data, peer) +threading.Thread(target=echo, daemon=True).start() +ms, fails = [], 0 +for _ in range(n): + c = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + c.settimeout(2) + t = time.perf_counter() + try: + c.sendto(b"x", srv.getsockname()) + c.recv(64) + ms.append((time.perf_counter() - t) * 1000) + except OSError: + fails += 1 + c.close() +r = {"label": label, "direction": "lo", "ok": len(ms), "failed": fails, "ms": None} +if len(ms) > 1: + q = statistics.quantiles(ms, n=100) + r["ms"] = {"mean": statistics.fmean(ms), "p50": q[49], "p90": q[89], + "p95": q[94], "p99": q[98], "max": max(ms)} +print(json.dumps(r)) +ECHO + q1="$(qseq)" + ctx "$1 queued_packets=$(( q1 - q0 )) conntrack=$(ctcount)" + [ "$2" != none ] && stop_daemon + return 0 +} + +measure() { # $1 label $2 n $3 mode(none|queue) $4 binary local label="$1" n="$2" mode="$3" bin="${4:-/usr/bin/colony-firewalld}" q0 q1 say "state: $label n=$n mode=$mode daemon=$(basename "$bin")" if [ "$mode" != none ]; then [ -x "$bin" ] || { echo "SKIP $label: $bin is not in this image"; return 0; } - if [ "$mode" = fast ]; then write_cfg true; else write_cfg false; fi - start_daemon "$bin" || { echo "FAIL $label"; return 1; } - nft -f "$SNIPPET" || { echo "FAIL $label nft"; stop_daemon; return 1; } - write_rules - cfc --socket "$SOCK" rules import --replace /tmp/rules.json >/dev/null 2>&1 - sleep 4 - ctx "$label fast_allow=$(cfc --socket "$SOCK" status --json 2>/dev/null | python3 -c 'import json,sys; print(json.load(sys.stdin).get("fast_allow","?"))' 2>/dev/null || echo unreachable)" + arm "$label" "$bin" || return 1 fi ctx "$label before sockets=$(sockets) conntrack=$(ctcount)" q0="$(qseq)" @@ -137,8 +181,7 @@ done for n in "$SMALL" "$LARGE"; do drain; measure "poll200us-$n" "$n" queue /usr/bin/colony-firewalld-alt done -for n in "$SMALL" "$LARGE"; do - drain; measure "fast-$n" "$n" fast /usr/bin/colony-firewalld -done [ "$SMALL" != "$LARGE" ] && { drain; measure "floor-$LARGE" "$LARGE" none; } +drain; measure_lo lo-floor none +drain; measure_lo lo-queue queue say "done" diff --git a/scripts/vm-bench/report.py b/scripts/vm-bench/report.py index 65a7729..75d7937 100755 --- a/scripts/vm-bench/report.py +++ b/scripts/vm-bench/report.py @@ -56,7 +56,7 @@ def counts(prefix): return sorted(int(k.rsplit("-", 1)[1]) for k in out if k.rsplit("-", 1)[0] == prefix and k.rsplit("-", 1)[1].isdigit()) - q, f, fl, po = (counts(x) for x in ("queue", "fast", "floor", "poll200us")) + q, fl, po = (counts(x) for x in ("queue", "floor", "poll200us")) print("\nreadings (p50 of the `out` direction, the one that meets the queue)") if len(q) > 1: pair(f"queue-{q[-1]}", f"queue-{q[0]}", @@ -64,10 +64,6 @@ def counts(prefix): for n in po: pair(f"poll200us-{n}", f"queue-{n}", f"{n} flows: a 200us idle beat against the 5ms one") - for n in f: - pair(f"fast-{n}", f"queue-{n}", f"{n} flows: the fast path against the queue") - for n in sorted(set(f) & set(fl)): - pair(f"fast-{n}", f"floor-{n}", f"{n} flows: what the fast path costs over nothing") if len(fl) > 1: pair(f"floor-{fl[-1]}", f"floor-{fl[0]}", f"the floor itself, {fl[-1]} flows against {fl[0]}") diff --git a/systemd/colony-firewalld.service b/systemd/colony-firewalld.service index b0dca18..12378d1 100644 --- a/systemd/colony-firewalld.service +++ b/systemd/colony-firewalld.service @@ -54,8 +54,9 @@ TimeoutStopSec=15 # without CAP_DAC_READ_SEARCH every connection resolves to `exe= # pid=0`, so no exe-scoped rule can ever match # and a fail-closed ruleset denies the whole -# machine's traffic. No warning at all: from the -# daemon's side, nothing failed. +# machine's non-loopback traffic. No warning +# at all: from the daemon's side, nothing +# failed. # # CAP_DAC_OVERRIDE is deliberately NOT granted: read-and-search is all this # needs, and the write half is what would let the daemon edit files it does not @@ -210,10 +211,11 @@ MemoryDenyWriteExecute=true # else about this unit is compatible with that child - ProtectSystem=strict # leaves /var/lib/rpm readable, and rpm needs no writable-executable memory. If # it ever is not compatible, the failure is a warning and an empty provenance -# index, never a failure to filter. The other child is nft(8), run at start -# and at stop to flush legacy Fast Allow state. The same fork/execve and -# CAP_NET_ADMIN netlink operations are needed. Failed cleanup is reported -# explicitly because an older installed ruleset may still accept old marks. +# index, never a failure to filter. The other child is nft(8), run once at +# start to flush the legacy Fast Allow set and once a minute to probe the +# table. The same fork/execve and CAP_NET_ADMIN netlink operations are needed. +# A failed flush is reported explicitly because an older installed ruleset +# may still accept old marks. SystemCallFilter=@system-service SystemCallFilter=bpf perf_event_open SystemCallErrorNumber=EPERM diff --git a/systemd/daemon.toml.sample b/systemd/daemon.toml.sample index 46a0a88..bc34c3c 100644 --- a/systemd/daemon.toml.sample +++ b/systemd/daemon.toml.sample @@ -87,9 +87,9 @@ profile = "balanced" [nfqueue] # NFQUEUE number. Must match the `queue num N` in the nftables/iptables rule # that enqueues packets. If they disagree, packets queue to a number nobody -# consumes - which under the shipped fail-closed rule is a total outbound -# lockout. Bind failure is fatal: the daemon exits non-zero rather than -# running while enforcing nothing. +# consumes - which under the shipped fail-closed rule is an outbound lockout +# for everything except new loopback flows. Bind failure is fatal: the +# daemon exits non-zero rather than running while enforcing nothing. queue_num = 0 # Kernel queue length: how many packets may wait for a verdict before the # queue overflows. Raise it on a busy host that prompts a lot; each queued @@ -206,10 +206,9 @@ enabled = true # # # # WHAT IT DOES NOT DO # # A process-wide Deny can refuse connect() in the kernel. Allow decisions and -# # conditional rules still use NFQUEUE. Fast Allow is disabled in every -# # configuration because a socket mark cannot identify the current sender. -# # Neither this layer nor NFQUEUE confines inherited sockets, local relays or -# # packet-layer traffic with CAP_NET_RAW. +# # conditional rules still use NFQUEUE. Neither this layer nor NFQUEUE +# # confines inherited sockets, local relays or packet-layer traffic with +# # CAP_NET_RAW. # # # # REQUIREMENTS - all of which degrade to a warning, never to a failure to # # start. If any of this is missing the daemon logs what it could not do and @@ -238,12 +237,8 @@ enabled = true # # /usr/lib/colony-firewall/cfc-ebpf.o. Point it at # # `cargo xtask ebpf-path` output when working on the programs themselves. # object_path = "/usr/lib/colony-firewall/cfc-ebpf.o" -# # Compatibility settings retained for existing configuration files. -# # Fast Allow is disabled. Setting true logs a warning and creates no grants; -# # allowed connections continue through NFQUEUE. Keep this false. -# fast_allow = false -# # An old configured mark has no runtime effect while Fast Allow is disabled. -# fast_allow_mark = 0x00033331 +# # fast_allow and fast_allow_mark are legacy keys from the removed Fast Allow +# # path: ignored, and a warning is logged if either is set. Delete them. # Needs a restart (the socket is secured once, right after bind). [ipc] diff --git a/systemd/nftables-inbound.conf b/systemd/nftables-inbound.conf index dbc3f6c..0ba22b2 100644 --- a/systemd/nftables-inbound.conf +++ b/systemd/nftables-inbound.conf @@ -47,10 +47,12 @@ table inet colony_firewall_inbound { # is the point. type filter hook input priority 0; policy drop; - # Loopback, first and unconditionally. The same reasoning as the - # outbound chain: the systemd-resolved stub, every local IPC over TCP - # and the daemon's own control socket live here, and filtering them - # buys nothing while being an excellent way to wedge the machine. + # Loopback, first and unconditionally. Every loopback flow also leaves + # through the outbound chain, which is where policy applies to it (the + # daemon judges it while it runs). Filtering the inbound copy as well + # buys nothing: the systemd-resolved stub, every local IPC over TCP and + # the daemon's own control socket live here, and a second drop point is + # an excellent way to wedge the machine. iifname "lo" accept # Traffic we asked for. Without this every reply to an outbound diff --git a/systemd/nftables-snippet.conf b/systemd/nftables-snippet.conf index 48a0579..5b42e2d 100644 --- a/systemd/nftables-snippet.conf +++ b/systemd/nftables-snippet.conf @@ -1,6 +1,9 @@ -# New outbound flows require a verdict from NFQUEUE 0, without bypass. -# Established flows retain their connection-wide authorization, including SSH -# replies. Loopback is outside application policy; see docs/HARDENING.md. +# New outbound flows require a verdict from NFQUEUE 0. Fail-closed for +# everything except new loopback flows, which are allowed while no daemon +# listens on the queue. Established flows retain their connection-wide +# authorization, including SSH replies. While the daemon runs, new loopback +# flows follow explicit application policy; unmatched local IPC is allowed +# without prompting. See docs/HARDENING.md. # Enable colony-firewall-nft.service for persistence. Its rules remain loaded # across daemon restarts and stops. Stop that unit explicitly to lift filtering. @@ -18,7 +21,6 @@ table inet colony_firewall { chain output { type filter hook output priority 0; policy drop; - oifname "lo" accept ct state established,related accept # The daemon's dedicated Reject sockets stamp this value. The root UID @@ -28,6 +30,11 @@ table inet colony_firewall { meta skuid 0 meta mark 0xcfc00001 icmp type destination-unreachable accept meta skuid 0 meta mark 0xcfc00001 icmpv6 type destination-unreachable accept + # Loopback is judged by the daemon while it runs. If the daemon is down, + # bypass lets local IPC keep working, including the systemd-resolved + # stub socket (cached names only: its upstream queries are not loopback); + # every other new flow falls through to the fail-closed rule below. + oifname "lo" ct state new queue num 0 bypass ct state new queue num 0 # INVALID and UNTRACKED traffic drops, including explicit notrack flows. }