diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 36cd360..6c6ea08 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -69,7 +69,7 @@ jobs: env: RUSTFLAGS: "" run: | - for target in message message_body attrs policy prefix_text config rtr netlink; do + for target in message message_body attrs prefix_text config rtr netlink; do mkdir -p "corpus/$target" cargo fuzz run "$target" "corpus/$target" "seeds/$target" -- -max_total_time=60 done diff --git a/Cargo.lock b/Cargo.lock index 1d52c18..b15ce89 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8,19 +8,6 @@ version = "2.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "320119579fcad9c21884f5c4861d16174d0e06250625266f50fe6898340abefa" -[[package]] -name = "ahash" -version = "0.8.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a15f179cd60c4584b8a8c596927aadc462e27f2ca70c04e0071964a73ba7a75" -dependencies = [ - "cfg-if", - "const-random", - "once_cell", - "version_check", - "zerocopy", -] - [[package]] name = "aho-corasick" version = "1.1.4" @@ -172,7 +159,6 @@ checksum = "ac07cdecf99051d9a5238b80f35af32cdeba5b336e55d957b318b50137e18da5" name = "bgp" version = "0.0.2" dependencies = [ - "bgp-policy", "bgp-server", "bgp-web", "clap", @@ -189,7 +175,6 @@ version = "0.0.2" dependencies = [ "bgp-integration-tests", "bgp-packet", - "bgp-policy", "bgp-primitives", "bgp-rib", "bgp-server", @@ -225,7 +210,6 @@ version = "0.0.2" dependencies = [ "bgp-fib", "bgp-packet", - "bgp-policy", "bgp-primitives", "bgp-rib", "bgp-rpki", @@ -255,18 +239,6 @@ dependencies = [ "thiserror 2.0.12", ] -[[package]] -name = "bgp-policy" -version = "0.0.2" -dependencies = [ - "bgp-packet", - "bgp-primitives", - "foldhash", - "rhai", - "thiserror 2.0.12", - "tracing", -] - [[package]] name = "bgp-primitives" version = "0.0.2" @@ -313,7 +285,6 @@ version = "0.0.2" dependencies = [ "bgp-fib", "bgp-packet", - "bgp-policy", "bgp-primitives", "bgp-rib", "bgp-rpki", @@ -460,26 +431,6 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b05b61dc5112cbb17e4b6cd61790d9845d13888356391624cbe7e41efeac1e75" -[[package]] -name = "const-random" -version = "0.1.18" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87e00182fe74b066627d63b85fd550ac2998d4b0bd86bfed477a0ae4c7c71359" -dependencies = [ - "const-random-macro", -] - -[[package]] -name = "const-random-macro" -version = "0.1.16" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f9d839f2a20b0aee515dc581a6172f2321f96cab76c1a38a4c584a194955390e" -dependencies = [ - "getrandom 0.2.17", - "once_cell", - "tiny-keccak", -] - [[package]] name = "crc32fast" version = "1.5.2" @@ -489,12 +440,6 @@ dependencies = [ "cfg-if", ] -[[package]] -name = "crunchy" -version = "0.2.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "460fbee9c2c2f33933d720630a6a0bac33ba7053db5344fac858d4b8952d77d5" - [[package]] name = "displaydoc" version = "0.2.7" @@ -664,17 +609,6 @@ dependencies = [ "slab", ] -[[package]] -name = "getrandom" -version = "0.2.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ff2abc00be7fca6ebc474524697ae276ad847ad0a6b3faa4bcb027e9a4614ad0" -dependencies = [ - "cfg-if", - "libc", - "wasi", -] - [[package]] name = "getrandom" version = "0.3.4" @@ -1086,15 +1020,6 @@ dependencies = [ "memoffset", ] -[[package]] -name = "no-std-compat" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b93853da6d84c2e3c7d730d6473e8817692dd89be387eb01b94d7f108ecb5b8c" -dependencies = [ - "spin", -] - [[package]] name = "nom" version = "8.0.0" @@ -1127,9 +1052,6 @@ name = "once_cell" version = "1.21.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d75b0bedcc4fe52caa0e03d9f1151a323e4aa5e2d78ba3580400cd3c9e2bc4bc" -dependencies = [ - "portable-atomic", -] [[package]] name = "once_cell_polyfill" @@ -1172,12 +1094,6 @@ version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3b3cff922bd51709b605d9ead9aa71031d81447142d828eb4a6eba76fe619f9b" -[[package]] -name = "portable-atomic" -version = "1.15.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "05c8b63e8d9609db387f0324918f81d68fe27748f084ef092fb35954d0539a85" - [[package]] name = "potential_utf" version = "0.1.6" @@ -1337,7 +1253,7 @@ version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "76afc826de14238e6e8c374ddcc1fa19e374fd8dd986b0d2af0d02377261d83c" dependencies = [ - "getrandom 0.3.4", + "getrandom", ] [[package]] @@ -1418,35 +1334,6 @@ dependencies = [ "web-sys", ] -[[package]] -name = "rhai" -version = "1.26.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0334639972c0ea5a3fd366aa36116754a11431b619fec3ed559b3f73bcbcebf5" -dependencies = [ - "ahash", - "bitflags", - "no-std-compat", - "num-traits", - "once_cell", - "rhai_codegen", - "smallvec", - "smartstring", - "thin-vec", - "web-time", -] - -[[package]] -name = "rhai_codegen" -version = "3.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3cd3a7535e50bf36857e7be7bec276d334e8c2dfa469c2201226fd01638ea5ca" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.100", -] - [[package]] name = "rustix" version = "0.38.44" @@ -1616,17 +1503,6 @@ version = "1.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7fcf8323ef1faaee30a44a340193b1ac6814fd9b7b4e88e9d4519a3e4abe1cfd" -[[package]] -name = "smartstring" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3fb72c633efbaa2dd666986505016c32c3044395ceaf881518399d2f4127ee29" -dependencies = [ - "autocfg", - "static_assertions", - "version_check", -] - [[package]] name = "socket2" version = "0.6.5" @@ -1637,24 +1513,12 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "spin" -version = "0.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6e63cff320ae2c57904679ba7cb63280a3dc4613885beafb148ee7bf9aa9042d" - [[package]] name = "stable_deref_trait" version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" -[[package]] -name = "static_assertions" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" - [[package]] name = "stats_alloc" version = "0.1.10" @@ -1738,18 +1602,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2d31c77bdf42a745371d260a26ca7163f1e0924b64afa0b688e61b5a9fa02f16" dependencies = [ "fastrand", - "getrandom 0.3.4", + "getrandom", "once_cell", "rustix 1.1.2", "windows-sys 0.61.2", ] -[[package]] -name = "thin-vec" -version = "0.2.20" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4568d7e143ec86d2021c338bae2afa88699e84b8e0af523626654fe6f03a1748" - [[package]] name = "thiserror" version = "1.0.69" @@ -1799,15 +1657,6 @@ dependencies = [ "cfg-if", ] -[[package]] -name = "tiny-keccak" -version = "2.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2c9d3793400a45f954c52e73d068316d76b6f4e36977e3fcebb13a2721e80237" -dependencies = [ - "crunchy", -] - [[package]] name = "tinystr" version = "0.8.4" @@ -2027,12 +1876,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" -[[package]] -name = "version_check" -version = "0.9.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" - [[package]] name = "wait-timeout" version = "0.2.1" @@ -2131,16 +1974,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "web-time" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a6580f308b1fad9207618087a65c04e7a10bc77e02c8e84e9b00dd4b12fa0bb" -dependencies = [ - "js-sys", - "wasm-bindgen", -] - [[package]] name = "which" version = "4.4.2" diff --git a/Cargo.toml b/Cargo.toml index 1573450..9329b3d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,7 +29,6 @@ unimplemented = "warn" # --- Our crates --- bgp-fib = { path = "crates/fib" } bgp-packet = { path = "crates/packet" } -bgp-policy = { path = "crates/policy" } bgp-primitives = { path = "crates/primitives" } bgp-rib = { path = "crates/rib" } bgp-rpki = { path = "crates/rpki" } diff --git a/README.md b/README.md index 1b71590..3b1fff1 100644 --- a/README.md +++ b/README.md @@ -16,7 +16,7 @@ It has not yet run in a production network. ### Not there yet -- Announcing learned routes to other peers (the export policy for it is there: scripts) +- Announcing learned routes to other peers, and an export policy for it - TCP-AO / TCP MD5 and GTSM - Graceful restart, BFD, changing the configuration without a restart - Next hops that are not directly connected @@ -30,7 +30,6 @@ It has not yet run in a production network. | `crates/rib` | The routing table, best route selection, the journal of changes | | `crates/rpki` | The RTR client; origin validation and ASPA path verification | | `crates/fib` | The best routes in the Linux kernel, over netlink | -| `crates/policy` | The operator's own rules for routes, as Rhai scripts | | `crates/server` | Sessions, the state machine, policy, configuration | | `crates/web` | The status page, HTTP API and metrics | | `bin/bgp` | The daemon | diff --git a/bin/bgp/Cargo.toml b/bin/bgp/Cargo.toml index 2202d22..383a248 100644 --- a/bin/bgp/Cargo.toml +++ b/bin/bgp/Cargo.toml @@ -9,7 +9,6 @@ repository.workspace = true version.workspace = true [dependencies] -bgp-policy.workspace = true bgp-server.workspace = true bgp-web.workspace = true clap.workspace = true diff --git a/bin/bgp/README.md b/bin/bgp/README.md index e2b119a..2fd70d1 100644 --- a/bin/bgp/README.md +++ b/bin/bgp/README.md @@ -42,9 +42,8 @@ connections, with RFC 4271 collision detection), announces its originated routes to every peer with itself as next hop, judges what peers send by each peer's import policy, keeps what passes, chooses a best route for each prefix, and on Linux can put those best routes in the kernel so that the -machine forwards by them. The operator's own rules for what is taken, how -it is marked and ranked, and what each peer is told are a script. It does -not yet pass routes learned from one peer on to another. +machine forwards by them. It does not yet pass routes learned from one peer +on to another, and has no export policy. ## Who may connect @@ -90,112 +89,6 @@ A refused route is dropped, along with the route it would have replaced. `bgp_routes_rejected_total` counts them by reason, and the status page shows how many routes each peer has and how many were refused. -## Policy scripts - -The checks above say what no network should take from another. What this -network wants beyond that (which routes it prefers, what it marks them -with, what it tells which neighbor) is said in a script, in -[Rhai](https://rhai.rs). A function is given a route and the peer it comes -from or goes to, may change the route, and returns whether it is taken: - -```rust -const BLACKHOLE = "65535:666"; -const LEARNED_FROM = #{ customer: "64601:1000", peer: "64601:2000", provider: "64601:3000" }; -const PREFERENCE = #{ customer: 300, peer: 200, provider: 100 }; - -// Routes on their way in. -fn from_peer(route, peer) { - if route.has_community(BLACKHOLE) { return false; } - route.remove_community("64601:*"); // nobody else's word for our marks - route.add_community(LEARNED_FROM[peer.role]); // where we learned it - route.local_pref = PREFERENCE[peer.role]; - true -} - -// Routes on their way out. -fn to_peer(route, peer) { - if route.has_community("64601:3000") && peer.role != "customer" { return false; } - if peer.name == "backup-transit" { route.prepend(2); } - route.remove_community("64601:*"); - true -} -``` - -``` -policy { path: "policy.rhai" } # beside the configuration file -peers { - name: "transit-a" ... - role: PROVIDER - import { function: "from_peer" } # after every check of `import` - export { function: "to_peer" } -} -``` - -`policy { import_function: ... export_function: ... }` names the functions -for peers that name none. `testdata/policy.rhai` is a longer example. - -**What a function is given.** - -| | | -|---|---| -| `route.prefix`, `route.prefix_len`, `route.family` | `"203.0.113.0/24"`, `24`, `"ipv4"` | -| `route.within("203.0.112.0/22")` | whether the prefix is that one or inside it | -| `route.as_path`, `route.as_path_len` | `[64602, 3356, 15169]`, `3` | -| `route.origin_as`, `route.neighbor_as`, `route.path_has(3356)` | the last and first AS of the path; whether an AS is on it | -| `route.origin`, `route.next_hop` | `"igp"`, `"egp"` or `"incomplete"`; `"192.0.2.1"` | -| `route.communities`, `route.large_communities` | `["3356:3", ...]`, `["64601:1:2", ...]` | -| `route.has_community("3356:3")` | also `(3356, 3)`, and `"3356:*"` for any of that AS | -| `route.add_community(..)`, `route.remove_community("3356:*")` | `"*:*"` removes all | -| `route.has_large_community(..)`, `add_large_community(..)`, `remove_large_community(..)` | the same, as `"64601:1:2"` | -| `route.local_pref`, `route.med` | the number, or `()` for none; set with `route.local_pref = 200`, unset with `= ()` | -| `route.prepend(2)` | on the way out: our AS that many more times on the path | -| `peer.name`, `peer.asn`, `peer.address`, `peer.ibgp`, `peer.local_asn` | | -| `peer.role` | `"customer"`, `"provider"`, `"peer"`, `"route_server"`, `"route_server_client"`, or `""` | - -**How it fits in.** - -- On the way in, a function is asked after every check of the peer's - `import` has passed, and before the checks whose answer can change (the - RPKI's, and whether the next hop can be reached). A route it refuses is - counted as "refused by policy function". What it changes is what goes - into the table: a `local_pref` it sets is what route selection goes by. -- On the way out it is asked about every route the peer would be sent. The - daemon announces the routes it originates; it does not yet pass on what - it learned from other peers. When it does, those routes will go through - the same function, with the communities they were given on the way in. - LOCAL_PREF does not leave the AS whatever a function sets. -- The script is read when the daemon starts. One that does not parse, or a - function that is named and not there, stops it from starting. `bgp - --check config.textpb` reads the script and says which function is - whose. Changing the script takes a restart. -- A function that fails on a route (it asks for something that is not - there, or does not end) has refused that route: failing is not a way - through. `bgp_policy_failures_total{function}` counts these, and the log - says why, at most once a minute. -- A script can only compute. It has no files, no network and no clock, and - a function is stopped after 200,000 steps. - -**What it costs.** Rhai walks the script's syntax tree; a call like the -ones above takes one to three microseconds, where the general checks take -a fraction of one. So a function is not asked about every route. Routes -that arrive in one UPDATE share their attributes, and a function that does -not ask for the prefix (`prefix`, `prefix_len`, `within`) gives one answer -for all of them. One that does is asked about each. With one peer sending -a table of 1.23 million routes over a session (`cargo bench -p bgp-bench ---bench policy`, on a laptop): - -| | Time until all are in the table | -|---|---| -| no function | 0.37 s | -| a function that does nothing | 0.51 s | -| one that marks and ranks every route | 0.80 s | -| the same, and it looks at the prefix | 3.4 s | - -So: filter by prefix with `import { prefixes ... }` and the length limits, -which cost next to nothing, and keep scripts to what routes carry. A -function that has to look at the prefix still takes in a full table in a -few seconds. - ## Route origin validation (RPKI) With `rpki { caches { address: "..." port: 3323 } }` in the configuration, @@ -476,7 +369,6 @@ Routinator 0.15.2. What it covers: | `limits` | A peer whose role does not fit gets no session; a peer over its prefix limit gets Cease / Maximum Number of Prefixes and is held down | | `hijacks` | FRR announces our own prefix, a route whose next hop is BIRD, and the exchange LAN's own prefix: none is taken | | `nexthops` | A better route from FRR whose next hop cannot be reached is held and BIRD's is chosen and installed; they swap when the next hop becomes a neighbor, and swap back when it stops being one | -| `policy` | A policy script: routes marked with where they were learned and ranked by it, which changes which of FRR's and BIRD's route is chosen and installed; FRR told our route with a longer path and a mark, BIRD not told it | | `v6only` | The exchange LAN without IPv4 addresses: one IPv6 session with each router for both families, IPv4 routes with IPv6 next hops (RFC 8950) in BGP and in the kernel, IPv4 pings across | | `linklocal` | The exchange LAN with link-local addresses only, and the same ones on a second interface: sessions with FRR (by interface, with radvd beside the daemon) and BIRD at `fe80::…%ix0`, both families in BGP and in the kernel, pings across, the interface taken down and up | @@ -524,8 +416,6 @@ For alerting, `/metricz` has: | `bgp_peer_max_prefixes{peer}` | The peer's limit, where it has one | | `bgp_prefix_limit_exceeded_total{peer}` | Sessions ended for going over the limit | | `bgp_connections_refused_total{reason}` | Connections closed without a session | -| `bgp_policy_runs_total{direction}` | Times a policy function was asked about a route | -| `bgp_policy_failures_total{function}` | Times one failed, which refuses the route | | `bgp_peer_up{peer}` | 1 while the session with a configured peer is Established, else 0 | | `bgp_build_info{version}` | Always 1; says which version is running | @@ -564,11 +454,6 @@ groups: - alert: BgpHijackAttempt expr: increase(bgp_routes_rejected_total{reason=~"prefix is our own|next hop is not the peer"}[10m]) > 0 labels: { severity: page } - # A function of the policy script is failing: every route it fails - # on is refused. - - alert: BgpPolicyFunctionFailing - expr: increase(bgp_policy_failures_total[10m]) > 0 - labels: { severity: page } # The kernel is refusing routes, or its answers are being lost. - alert: BgpKernelErrors expr: increase(bgp_kernel_errors_total[10m]) > 0 diff --git a/bin/bgp/src/main.rs b/bin/bgp/src/main.rs index ec54df5..5c905b8 100644 --- a/bin/bgp/src/main.rs +++ b/bin/bgp/src/main.rs @@ -1,4 +1,4 @@ -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::sync::Arc; use bgp_server::config::Config; @@ -39,13 +39,7 @@ async fn main() -> Result<()> { .wrap_err_with(|| format!("Failed to read {}", args.config.display()))?; let proto_config = protobuf::text_format::parse_from_str::(&config_string) .wrap_err("Failed to parse config file")?; - // What the configuration names by a relative path is beside it. - let beside = args - .config - .parent() - .filter(|directory| !directory.as_os_str().is_empty()) - .unwrap_or(Path::new(".")); - let config = Config::from_proto_in(proto_config, beside).wrap_err("Invalid configuration")?; + let config = Config::from_proto(proto_config).wrap_err("Invalid configuration")?; if args.check { println!( "{} is in order: AS{}, peers: {}, routes originated: {}", @@ -54,22 +48,6 @@ async fn main() -> Result<()> { config.peers.len(), config.originate.len() ); - if let Some(policy) = &config.policy { - println!("policy functions: {}", policy.functions().join(", ")); - for peer in &config.peers { - let name = |hook: &Option| { - hook.as_ref() - .map_or("-", bgp_policy::Hook::name) - .to_string() - }; - println!( - " {}: import {}, export {}", - peer.name, - name(&peer.import_hook), - name(&peer.export_hook) - ); - } - } return Ok(()); } diff --git a/crates/bench/Cargo.toml b/crates/bench/Cargo.toml index c28027a..fd8bf36 100644 --- a/crates/bench/Cargo.toml +++ b/crates/bench/Cargo.toml @@ -18,7 +18,6 @@ stats_alloc = { version = "0.1.10", optional = true } [dev-dependencies] bgp-integration-tests.workspace = true -bgp-policy.workspace = true bgp-rib.workspace = true bgp-server.workspace = true tokio.workspace = true @@ -39,9 +38,5 @@ harness = false name = "real_table" harness = false -[[bench]] -name = "policy" -harness = false - [lints] workspace = true diff --git a/crates/bench/benches/policy.rs b/crates/bench/benches/policy.rs deleted file mode 100644 index 59d5e75..0000000 --- a/crates/bench/benches/policy.rs +++ /dev/null @@ -1,350 +0,0 @@ -//! What the operator's policy script costs, on a real table: the routes -//! one peer of a RIPE RIS route collector had announced to it, each with -//! the attributes it really has. -//! -//! Needs a dump in `crates/bench/data/` (see the README there) or at the -//! path in `BENCH_MRT`; without one it says so and does nothing. -//! -//! A function is asked about a route once for every UPDATE if it does not -//! look at the prefix, and once for every prefix if it does. How many -//! UPDATEs a table arrives in is the sender's business; the two ends are -//! one UPDATE for every distinct set of attributes, and one for every -//! route. Both are measured, with what the same work costs without a -//! script. - -use std::collections::HashSet; -use std::hint::black_box; -use std::path::PathBuf; -use std::time::{Duration, Instant}; - -use std::net::Ipv4Addr; - -use bgp_bench::mrt::{Dump, parse_attributes}; -use bgp_bench::{Prefixes, Shape, announcements, encoded}; -use bgp_integration_tests::test_peer::TestPeer; -use bgp_integration_tests::test_server::TestServer; -use bgp_packet::message::{NlriNextHop, PathAttribute}; -use bgp_policy::{Direction, Hook, Peer, Policy, Subject}; -use bgp_primitives::ip_prefix::IpPrefix; -use bgp_server::config::PeerConfig; -use bgp_server::family::FamilySet; -use bgp_server::policy::ImportPolicy; -use bytes::BytesMut; - -const SCRIPT: &str = r#" - const BLACKHOLE = "65535:666"; - const LEARNED_FROM = #{ customer: "64601:1000", peer: "64601:2000", provider: "64601:3000" }; - const PREFERENCE = #{ customer: 300, peer: 200, provider: 100 }; - - // Nothing at all: what asking costs. - fn nothing(route, peer) { true } - - // Looks, and changes nothing. - fn filter(route, peer) { - if route.has_community(BLACKHOLE) { return false; } - if route.as_path_len > 30 { return false; } - if route.path_has(64512) { return false; } - true - } - - // What a network would have for its transit: refuse some, mark and - // rank the rest. - fn typical(route, peer) { - if route.has_community(BLACKHOLE) { return false; } - if route.as_path_len > 30 { return false; } - route.remove_community("64601:*"); - route.add_community(LEARNED_FROM[peer.role]); - route.local_pref = PREFERENCE[peer.role]; - true - } - - // For any peer, whatever it is to us. - fn marks(route, peer) { - if route.has_community(BLACKHOLE) { return false; } - route.remove_community("64601:*"); - route.add_community("64601:3000"); - route.local_pref = 100; - true - } - - fn marks_by_prefix(route, peer) { - if route.prefix_len > 24 && route.family == "ipv4" { return false; } - marks(route, peer) - } - - // The same, and it looks at the prefix: asked for every route. - fn by_prefix(route, peer) { - if route.prefix_len > 24 && route.family == "ipv4" { return false; } - if route.has_community(BLACKHOLE) { return false; } - route.remove_community("64601:*"); - route.add_community(LEARNED_FROM[peer.role]); - route.local_pref = PREFERENCE[peer.role]; - true - } -"#; - -fn dump_path() -> Option { - if let Ok(path) = std::env::var("BENCH_MRT") { - return Some(path.into()); - } - let data = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("data"); - let mut dumps: Vec = std::fs::read_dir(data) - .ok()? - .filter_map(|entry| Some(entry.ok()?.path())) - .filter(|path| path.extension().is_some_and(|e| e == "gz")) - .collect(); - dumps.sort(); - dumps.pop() -} - -type Attributes = (Vec, Option); - -struct Outcome { - elapsed: Duration, - asked: usize, - taken: usize, - changed: usize, -} - -/// Asks `hook` about each of `routes`. -fn ask<'a>( - hook: &Hook, - peer: &Peer, - routes: impl Iterator, -) -> Outcome { - let started = Instant::now(); - let (mut asked, mut taken, mut changed) = (0, 0, 0); - for (prefix, (attributes, next_hop)) in routes { - let verdict = hook - .run( - peer, - &Subject { - prefix, - attributes, - next_hop: *next_hop, - direction: Direction::Import, - }, - ) - .expect("the function runs"); - asked += 1; - taken += usize::from(verdict.accept); - changed += usize::from(verdict.attributes.is_some()); - black_box(&verdict); - } - Outcome { - elapsed: started.elapsed(), - asked, - taken, - changed, - } -} - -fn row(what: &str, outcome: &Outcome) { - println!( - "{what:<52} {:>8.3} s {:>7.0} ns each {} asked, {} taken, {} changed", - outcome.elapsed.as_secs_f64(), - outcome.elapsed.as_nanos() as f64 / outcome.asked.max(1) as f64, - outcome.asked, - outcome.taken, - outcome.changed - ); -} - -/// One peer sends the whole (synthetic) table over TCP to a server that -/// gives its routes to `function`, or to none. Returns how long it took -/// for all of them to be in the table. -fn end_to_end(wire: &[BytesMut], routes: usize, function: Option<&str>) -> Duration { - const PEER_ASN: u32 = 64513; - let runtime = tokio::runtime::Builder::new_multi_thread() - .enable_all() - .build() - .expect("a runtime"); - runtime.block_on(async { - // The configured peer is whoever connects from this machine; the - // listener is where the server's own attempts to reach it go. - let listener = tokio::net::TcpListener::bind((Ipv4Addr::LOCALHOST, 0)) - .await - .expect("a port"); - let endpoint = listener.local_addr().expect("its address"); - let policy = Policy::compile(SCRIPT).expect("the script"); - let import_hook = function.map(|name| policy.hook(name).expect("the function")); - let server = TestServer::start_with(4_200_000_000, |config| { - config.accept_unconfigured_peers = false; - config.peers = vec![PeerConfig { - name: "transit".into(), - endpoint: endpoint.into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, - local_address: None, - families: FamilySet::supported(), - add_path: None, - import: ImportPolicy::permissive(), - role: None, - require_role: false, - import_hook, - export_hook: None, - }]; - }) - .await - .expect("a server"); - let mut session = TestPeer::connect(server.addr).await.expect("a connection"); - session - .handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 1), 90) - .await - .expect("a session"); - let stream: Vec = wire.iter().flat_map(|m| m.iter().copied()).collect(); - let started = Instant::now(); - let sending = tokio::spawn(async move { - session.send_raw(&stream).await.expect("the server reads"); - session - }); - while server.rib.len() < routes { - tokio::time::sleep(Duration::from_millis(2)).await; - } - let elapsed = started.elapsed(); - drop(sending.await); - elapsed - }) -} - -fn main() { - // The whole way, first: a session, its state machine, the general - // checks, the function, the table. On a table that is made up, so - // that it runs anywhere. - let shape = Shape::INTERNET; - let prefixes = Prefixes::generate(shape); - let wire = encoded(&announcements(&prefixes, shape, 1)); - let routes = shape.prefixes(); - println!( - "== End to end: one peer sends {routes} routes in {} UPDATEs ({:.1} routes each) over TCP", - wire.len(), - routes as f64 / wire.len() as f64 - ); - let mut without = Duration::ZERO; - for (what, function) in [ - ("no policy function", None), - ("a function that does nothing", Some("nothing")), - ("marks and ranks every route", Some("marks")), - ("the same, and looks at the prefix", Some("marks_by_prefix")), - ] { - // The better of two: the first run of all also warms the machine. - let elapsed = end_to_end(&wire, routes, function).min(end_to_end(&wire, routes, function)); - if function.is_none() { - without = elapsed; - } - println!( - "{what:<52} {:>8.3} s {:.2} M routes/s {:+.0}%", - elapsed.as_secs_f64(), - routes as f64 / elapsed.as_secs_f64() / 1e6, - 100.0 * (elapsed.as_secs_f64() / without.as_secs_f64() - 1.0) - ); - } - println!(); - - let Some(path) = dump_path() else { - println!("No table dump found; see crates/bench/data/README.md. Nothing more to do."); - return; - }; - println!("== Reading {}", path.display()); - let dump = Dump::read(&path).expect("a readable dump"); - let peer_index = dump.largest_peers(1)[0]; - let routes = &dump.routes[peer_index]; - let parsed: Vec> = dump - .attribute_sets - .iter() - .map(|wire| parse_attributes(wire)) - .collect(); - let routes: Vec<(&IpPrefix, &Attributes)> = routes - .iter() - .filter_map(|(prefix, set)| Some((prefix, parsed[*set as usize].as_ref()?))) - .collect(); - let distinct: HashSet<*const Attributes> = routes - .iter() - .map(|(_, attributes)| std::ptr::from_ref(*attributes)) - .collect(); - let mut seen = HashSet::new(); - let once_per_set: Vec<(&IpPrefix, &Attributes)> = routes - .iter() - .filter(|(_, attributes)| seen.insert(std::ptr::from_ref(*attributes))) - .copied() - .collect(); - let communities: usize = routes - .iter() - .map(|(_, (attributes, _))| { - attributes - .iter() - .map(|attribute| match attribute { - PathAttribute::Communities(communities) => communities.0.len(), - _ => 0, - }) - .sum::() - }) - .sum(); - println!( - "The peer with the most routes, AS{}: {} routes with {} distinct sets of attributes \ - ({:.1} routes a set), {:.1} communities a route\n", - dump.peers[peer_index].asn, - routes.len(), - distinct.len(), - routes.len() as f64 / distinct.len() as f64, - communities as f64 / routes.len() as f64 - ); - - let policy = Policy::compile(SCRIPT).expect("the script"); - let peer = Peer::new( - "transit", - dump.peers[peer_index].address, - dump.peers[peer_index].asn, - 64601, - "provider", - ); - - // What it costs to go over the routes and do the work of the typical - // function in Rust. - let started = Instant::now(); - let mut taken = 0usize; - for (_, (attributes, _)) in &routes { - let mut communities: Vec<(u16, u16)> = attributes - .iter() - .find_map(|attribute| match attribute { - PathAttribute::Communities(communities) => Some(communities.0.clone()), - _ => None, - }) - .unwrap_or_default(); - if communities.contains(&(65535, 666)) { - continue; - } - communities.retain(|community| community.0 != 64601); - communities.push((64601, 3000)); - let mut changed = attributes.clone(); - changed.push(PathAttribute::Communities( - bgp_packet::message::CommunitiesPathAttribute(communities), - )); - black_box(&changed); - taken += 1; - } - row( - "the typical one in Rust, for every route", - &Outcome { - elapsed: started.elapsed(), - asked: routes.len(), - taken, - changed: taken, - }, - ); - println!(); - - for function in ["nothing", "filter", "typical", "by_prefix"] { - let hook = policy.hook(function).expect("the function"); - row( - &format!("{function}, asked for every route"), - &ask(&hook, &peer, routes.iter().copied()), - ); - if function != "by_prefix" { - row( - &format!("{function}, asked once for each set of attributes"), - &ask(&hook, &peer, once_per_set.iter().copied()), - ); - } - } -} diff --git a/crates/integration-tests/Cargo.toml b/crates/integration-tests/Cargo.toml index 65805ad..da6d3cc 100644 --- a/crates/integration-tests/Cargo.toml +++ b/crates/integration-tests/Cargo.toml @@ -11,7 +11,6 @@ publish = false [dependencies] bgp-fib.workspace = true bgp-packet.workspace = true -bgp-policy.workspace = true bgp-primitives.workspace = true bgp-rib.workspace = true bgp-rpki.workspace = true diff --git a/crates/integration-tests/src/test_server.rs b/crates/integration-tests/src/test_server.rs index 1755d85..22fc9f0 100644 --- a/crates/integration-tests/src/test_server.rs +++ b/crates/integration-tests/src/test_server.rs @@ -67,7 +67,6 @@ impl TestServer { http_endpoint: None, rpki: None, kernel: None, - policy: None, }; customize(&mut config); diff --git a/crates/integration-tests/tests/add_path.rs b/crates/integration-tests/tests/add_path.rs index eeb530f..d9fdb4b 100644 --- a/crates/integration-tests/tests/add_path.rs +++ b/crates/integration-tests/tests/add_path.rs @@ -50,8 +50,6 @@ async fn server_offering( import: ImportPolicy::permissive(), role: None, require_role: false, - import_hook: None, - export_hook: None, }]; c.originate = originate; // Keep the dialer out of the way. diff --git a/crates/integration-tests/tests/admission.rs b/crates/integration-tests/tests/admission.rs index a268d2f..ff60c34 100644 --- a/crates/integration-tests/tests/admission.rs +++ b/crates/integration-tests/tests/admission.rs @@ -28,8 +28,6 @@ fn peer_at(endpoint: SocketAddr) -> PeerConfig { import: ImportPolicy::permissive(), role: None, require_role: false, - import_hook: None, - export_hook: None, } } diff --git a/crates/integration-tests/tests/aspa.rs b/crates/integration-tests/tests/aspa.rs index be0494a..44b21f6 100644 --- a/crates/integration-tests/tests/aspa.rs +++ b/crates/integration-tests/tests/aspa.rs @@ -67,8 +67,6 @@ async fn server_with( import, role, require_role: false, - import_hook: None, - export_hook: None, }]; }) .await?; diff --git a/crates/integration-tests/tests/collision.rs b/crates/integration-tests/tests/collision.rs index 2e9ce73..3f1e616 100644 --- a/crates/integration-tests/tests/collision.rs +++ b/crates/integration-tests/tests/collision.rs @@ -46,8 +46,6 @@ async fn server_dialing(listener: &TcpListener) -> Result { import: ImportPolicy::permissive(), role: None, require_role: false, - import_hook: None, - export_hook: None, }]; }) .await diff --git a/crates/integration-tests/tests/debugging.rs b/crates/integration-tests/tests/debugging.rs index 1adb415..8ef82a5 100644 --- a/crates/integration-tests/tests/debugging.rs +++ b/crates/integration-tests/tests/debugging.rs @@ -180,8 +180,6 @@ async fn test_refused_routes_can_be_seen_and_counted() -> Result<()> { }, role: None, require_role: false, - import_hook: None, - export_hook: None, }]; }) .await?; diff --git a/crates/integration-tests/tests/dialer.rs b/crates/integration-tests/tests/dialer.rs index a079289..44c1d9a 100644 --- a/crates/integration-tests/tests/dialer.rs +++ b/crates/integration-tests/tests/dialer.rs @@ -37,8 +37,6 @@ async fn server_dialing(listener: &TcpListener, remote_asn: u32) -> Result Result<()> { import: ImportPolicy::permissive(), role: None, require_role: false, - import_hook: None, - export_hook: None, }]; }) .await?; @@ -249,8 +245,6 @@ async fn test_local_address_is_used_as_the_source() -> Result<()> { import: ImportPolicy::permissive(), role: None, require_role: false, - import_hook: None, - export_hook: None, }]; }) .await?; @@ -271,8 +265,6 @@ async fn test_local_address_is_used_as_the_source() -> Result<()> { import: ImportPolicy::permissive(), role: None, require_role: false, - import_hook: None, - export_hook: None, }]; }) .await?; diff --git a/crates/integration-tests/tests/import_policy.rs b/crates/integration-tests/tests/import_policy.rs index 26e092c..69a1dd8 100644 --- a/crates/integration-tests/tests/import_policy.rs +++ b/crates/integration-tests/tests/import_policy.rs @@ -51,8 +51,6 @@ async fn server_with(import: ImportPolicy) -> Result<(TestServer, TcpListener)> import, role: None, require_role: false, - import_hook: None, - export_hook: None, }]; }) .await?; diff --git a/crates/integration-tests/tests/negotiation.rs b/crates/integration-tests/tests/negotiation.rs index 7dc7950..3dfa4ad 100644 --- a/crates/integration-tests/tests/negotiation.rs +++ b/crates/integration-tests/tests/negotiation.rs @@ -193,8 +193,6 @@ async fn test_no_family_in_common_refuses_the_session() -> Result<()> { import: ImportPolicy::permissive(), role: None, require_role: false, - import_hook: None, - export_hook: None, }]; // Keep the dialer out of the way. c.connect_retry = Duration::from_secs(3600); diff --git a/crates/integration-tests/tests/policy_script.rs b/crates/integration-tests/tests/policy_script.rs deleted file mode 100644 index a21f8c0..0000000 --- a/crates/integration-tests/tests/policy_script.rs +++ /dev/null @@ -1,361 +0,0 @@ -//! The operator's own rules, as a script: what they do to routes on their -//! way in, and to what a peer is told. - -use std::collections::BTreeMap; -use std::net::Ipv4Addr; -use std::time::Duration; - -use bgp_integration_tests::default_policy; -use bgp_integration_tests::test_peer::TestPeer; -use bgp_integration_tests::test_server::TestServer; -use bgp_packet::message::{ - AsPathAttribute, CommunitiesPathAttribute, LocalPrefPathAttribute, Message, - NextHopPathAttribute, OriginPathAttribute, PathAttribute, UpdateMessage, -}; -use bgp_policy::Policy; -use bgp_primitives::ip_prefix::IpPrefix; -use bgp_rib::{LocalRoute, LocalRouteAttrs}; -use bgp_server::config::PeerConfig; -use bgp_server::family::FamilySet; -use bgp_server::policy::PeerRole; -use eyre::Result; -use tokio::net::TcpListener; - -const LOCAL_ASN: u32 = 1299; -const PEER_ASN: u32 = 3333; -const TIMEOUT: Duration = Duration::from_secs(5); - -const SCRIPT: &str = r#" - const BLACKHOLE = "65535:666"; - const LEARNED_FROM = #{ customer: "1299:1000", peer: "1299:2000", provider: "1299:3000" }; - - fn from_peer(route, peer) { - if route.has_community(BLACKHOLE) { return false; } - // Nobody else's word for where we learned a route. - route.remove_community("1299:*"); - route.add_community(LEARNED_FROM[peer.role]); - route.local_pref = if route.has_community("3333:80") { 80 } else { 200 }; - true - } - - fn by_prefix(route, peer) { - if route.within("194.0.0.0/8") { return false; } - if route.prefix == "193.0.4.0/24" { route.add_community("1299:4"); } - true - } - - fn broken(route, peer) { - if route.prefix_len == 22 { return route.no_such_thing; } - true - } - - fn to_peer(route, peer) { - if route.has_community("1299:9") { return false; } // not for anyone else - if route.within("193.0.0.0/21") { route.prepend(2); } - route.add_community("1299:" + peer.asn % 1000); - route.med = 50; - true - } -"#; - -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} - -/// A server with one configured peer, whose routes are given to the -/// script's function `import`, and which is told what `export` lets by. -async fn server_with( - import: Option<&str>, - export: Option<&str>, - originate: Vec, -) -> Result<(TestServer, TcpListener)> { - let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; - let endpoint = listener.local_addr()?; - let policy = Policy::compile(SCRIPT)?; - let import_hook = import.map(|name| policy.hook(name)).transpose()?; - let export_hook = export.map(|name| policy.hook(name)).transpose()?; - let server = TestServer::start_with(LOCAL_ASN, |c| { - c.accept_unconfigured_peers = false; - c.originate = originate; - c.policy = Some(policy); - c.peers = vec![PeerConfig { - name: "r1".into(), - endpoint: endpoint.into(), - remote_asn: PEER_ASN, - hold_time_secs: 90, - local_address: None, - families: FamilySet::supported(), - add_path: None, - import: default_policy(), - role: Some(PeerRole::Peer), - require_role: false, - import_hook, - export_hook, - }]; - }) - .await?; - Ok((server, listener)) -} - -async fn established(server: &TestServer) -> Result { - let mut peer = TestPeer::connect(server.addr).await?; - peer.handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90) - .await?; - tokio::time::timeout(TIMEOUT, async { - while server.registry.list().is_empty() { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await?; - Ok(peer) -} - -async fn announce( - peer: &mut TestPeer, - prefixes: &[&str], - communities: &[(u16, u16)], -) -> Result<()> { - let mut path_attributes = vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![PEER_ASN]), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), - ]; - if !communities.is_empty() { - path_attributes.push(PathAttribute::Communities(CommunitiesPathAttribute( - communities.to_vec(), - ))); - } - peer.send(&Message::Update(UpdateMessage { - withdrawn_routes: vec![], - path_attributes, - announced_routes: prefixes.iter().map(|p| prefix(p).into()).collect(), - })) - .await -} - -/// Waits until the server has taken `taken` routes of the session's and -/// refused `refused`. -async fn dealt_with(server: &TestServer, taken: u64, refused: u64) -> Result<()> { - let counters = server.registry.list()[0].counters.clone(); - tokio::time::timeout(TIMEOUT, async { - while (counters.routes(), counters.rejected()) != (taken, refused) { - tokio::time::sleep(Duration::from_millis(5)).await; - } - }) - .await - .map_err(|_| { - eyre::eyre!( - "the server took {} routes and refused {}; expected {taken} and {refused}", - counters.routes(), - counters.rejected() - ) - }) -} - -/// A route's communities and its LOCAL_PREF. -type Marks = (Vec<(u16, u16)>, Option); - -/// The communities and LOCAL_PREF each prefix is in the table with. -fn table(server: &TestServer) -> BTreeMap { - server - .received_routes() - .iter() - .map(|route| { - let attributes = &route.attributes; - let communities = attributes - .iter() - .find_map(|attribute| match attribute { - PathAttribute::Communities(communities) => Some(communities.0.clone()), - _ => None, - }) - .unwrap_or_default(); - let local_pref = attributes.iter().find_map(|attribute| match attribute { - PathAttribute::LocalPref(LocalPrefPathAttribute(value)) => Some(*value), - _ => None, - }); - (route.prefix.to_string(), (communities, local_pref)) - }) - .collect() -} - -fn refusals(server: &TestServer) -> Vec<(&'static str, u64)> { - server.refusals.peers()[0] - .1 - .by_reason() - .into_iter() - .map(|(reason, count)| (reason.as_str(), count)) - .collect() -} - -/// Routes are marked with where they were learned and given a -/// preference, by what they carry and who sent them; one the function -/// refuses is not taken. -#[tokio::test] -async fn test_routes_are_marked_preferred_and_refused_on_their_way_in() -> Result<()> { - let (server, _listener) = server_with(Some("from_peer"), None, vec![]).await?; - let mut peer = established(&server).await?; - - // One UPDATE of two routes; one of a route with marks of its own, one - // of them forged; and one the function refuses. - announce(&mut peer, &["193.0.0.0/21", "193.0.8.0/22"], &[]).await?; - announce(&mut peer, &["194.0.1.0/24"], &[(3333, 80), (1299, 1000)]).await?; - announce(&mut peer, &["195.0.1.0/24"], &[(65535, 666)]).await?; - dealt_with(&server, 3, 1).await?; - - assert_eq!( - table(&server), - BTreeMap::from([ - ("193.0.0.0/21".to_string(), (vec![(1299, 2000)], Some(200))), - ("193.0.8.0/22".to_string(), (vec![(1299, 2000)], Some(200))), - ( - "194.0.1.0/24".to_string(), - (vec![(3333, 80), (1299, 2000)], Some(80)) - ), - ]) - ); - assert_eq!(refusals(&server), [("refused by policy function", 1)]); - - // A route the function took is replaced by one it refuses: gone. - announce(&mut peer, &["193.0.8.0/22"], &[(65535, 666)]).await?; - dealt_with(&server, 2, 2).await?; - assert!(!table(&server).contains_key("193.0.8.0/22")); - Ok(()) -} - -/// A function that asks for the prefix is asked about each route of an -/// UPDATE, and may answer differently for each. -#[tokio::test] -async fn test_a_function_may_treat_the_routes_of_one_update_differently() -> Result<()> { - let (server, _listener) = server_with(Some("by_prefix"), None, vec![]).await?; - let mut peer = established(&server).await?; - announce( - &mut peer, - &[ - "193.0.0.0/21", - "194.0.1.0/24", - "193.0.4.0/24", - "194.0.2.0/24", - "195.0.0.0/24", - ], - &[(3333, 1)], - ) - .await?; - dealt_with(&server, 3, 2).await?; - assert_eq!( - table(&server), - BTreeMap::from([ - ("193.0.0.0/21".to_string(), (vec![(3333, 1)], None)), - ( - "193.0.4.0/24".to_string(), - (vec![(3333, 1), (1299, 4)], None) - ), - ("195.0.0.0/24".to_string(), (vec![(3333, 1)], None)), - ]) - ); - assert_eq!( - server.rib.attrs().len(), - 2, - "two routes share their attributes" - ); - Ok(()) -} - -/// A function that fails on a route has refused it, and no other. -#[tokio::test] -async fn test_a_function_that_fails_refuses_the_route() -> Result<()> { - let (server, _listener) = server_with(Some("broken"), None, vec![]).await?; - let mut peer = established(&server).await?; - announce( - &mut peer, - &["193.0.0.0/21", "193.0.8.0/22", "195.0.0.0/24"], - &[], - ) - .await?; - dealt_with(&server, 2, 1).await?; - assert_eq!(refusals(&server), [("policy function failed", 1)]); - assert!(!table(&server).contains_key("193.0.8.0/22")); - Ok(()) -} - -/// What a peer is told is what the function for it lets by, as the -/// function left it. -#[tokio::test] -async fn test_what_a_peer_is_told_goes_through_its_function() -> Result<()> { - let route = |prefix_text: &str, communities: Vec<(u16, u16)>| { - LocalRoute::new( - prefix(prefix_text), - LocalRouteAttrs { - communities, - ..LocalRouteAttrs::default() - }, - ) - }; - let originate = vec![ - route("193.0.0.0/21", vec![(1299, 1)]), - route("194.0.0.0/22", vec![]), - route("195.0.0.0/22", vec![(1299, 9)]), - ]; - let (server, _listener) = server_with(None, Some("to_peer"), originate).await?; - let mut peer = established(&server).await?; - - let mut told = BTreeMap::new(); - for _ in 0..2 { - let update = tokio::time::timeout(TIMEOUT, peer.recv_update()).await??; - let path = update - .path_attributes - .iter() - .find_map(|attribute| match attribute { - PathAttribute::AsPath(path) => Some(path.segments[0].path.clone()), - _ => None, - }) - .unwrap(); - let communities = update - .path_attributes - .iter() - .find_map(|attribute| match attribute { - PathAttribute::Communities(communities) => Some(communities.0.clone()), - _ => None, - }) - .unwrap_or_default(); - let med = update - .path_attributes - .iter() - .find_map(|attribute| match attribute { - PathAttribute::MultiExitDisc(med) => Some(med.0), - _ => None, - }); - // In the order of their type codes, as any UPDATE of ours. - let codes: Vec = update - .path_attributes - .iter() - .map(PathAttribute::type_code) - .collect(); - assert!(codes.is_sorted(), "{codes:?}"); - // The peer is one: the route is still marked as only for customers. - assert!(codes.contains(&35), "{codes:?}"); - told.insert( - update.announced_routes[0].prefix.to_string(), - (path, communities, med), - ); - } - assert_eq!( - told, - BTreeMap::from([ - ( - "193.0.0.0/21".to_string(), - (vec![LOCAL_ASN; 3], vec![(1299, 1), (1299, 333)], Some(50)) - ), - ( - "194.0.0.0/22".to_string(), - (vec![LOCAL_ASN], vec![(1299, 333)], Some(50)) - ), - ]) - ); - // And nothing of the third. - assert!( - tokio::time::timeout(Duration::from_millis(300), peer.recv_update()) - .await - .is_err() - ); - Ok(()) -} diff --git a/crates/integration-tests/tests/roles.rs b/crates/integration-tests/tests/roles.rs index 882710e..d66e085 100644 --- a/crates/integration-tests/tests/roles.rs +++ b/crates/integration-tests/tests/roles.rs @@ -58,8 +58,6 @@ async fn server_with(role: Option, require_role: bool) -> Result) -> std::fmt::Result { - f.debug_struct("Policy") - .field("functions", &self.functions()) - .finish() - } -} - -impl Policy { - /// Reads `source`. What it has outside of functions is run once, now: - /// that is where its constants are defined. - pub fn compile(source: &str) -> Result, PolicyError> { - let mut engine = Engine::new(); - engine.set_max_operations(MAX_OPERATIONS); - engine.set_max_call_levels(32); - engine.set_max_expr_depths(64, 32); - engine.set_max_string_size(16 * 1024); - engine.set_max_array_size(16 * 1024); - engine.set_max_map_size(1024); - // What a script prints is for whoever is writing it. - engine.on_print(|text| tracing::info!(target: "bgp_policy::script", "{text}")); - engine.on_debug(|text, _, position| { - tracing::debug!(target: "bgp_policy::script", %position, "{text}"); - }); - route::register(&mut engine); - - let ast = engine - .compile(source) - .map_err(|e| PolicyError::Parse(e.to_string()))?; - let mut globals = Scope::new(); - engine - .run_ast_with_scope(&mut globals, &ast) - .map_err(|e| PolicyError::Load(e.to_string()))?; - // A Rhai function sees nothing outside itself. A policy's - // constants are for all its functions, so they are found for - // them: this is asked for every variable a function names, before - // the function's own are looked at. - let constants: HashMap = globals - .iter() - .map(|(name, _, value)| (name.to_string(), value)) - .collect(); - if !constants.is_empty() { - #[allow(deprecated, reason = "the API is marked volatile, not obsolete")] - engine.on_var(move |name, _, _| Ok(constants.get(name).cloned())); - } - Ok(Arc::new(Self { engine, ast })) - } - - /// The names of the script's functions, in order. - pub fn functions(&self) -> Vec { - let mut names: Vec = self - .ast - .iter_functions() - .map(|function| function.name.to_string()) - .collect(); - names.sort(); - names.dedup(); - names - } - - /// The function called `name`, to be run on routes. - pub fn hook(self: &Arc, name: &str) -> Result { - let found: Vec = self - .ast - .iter_functions() - .filter(|function| function.name == name) - .map(|function| function.params.len()) - .collect(); - match found.as_slice() { - [] => Err(PolicyError::NoSuchFunction { - name: name.to_string(), - known: self.functions().join(", "), - }), - takes if takes.contains(&2) => Ok(Hook { - policy: Arc::clone(self), - name: name.to_string(), - }), - [takes, ..] => Err(PolicyError::Arguments { - name: name.to_string(), - takes: *takes, - }), - } - } -} - -/// A route as a policy is asked about it. -#[derive(Debug, Clone, Copy)] -pub struct Subject<'a> { - pub prefix: &'a IpPrefix, - /// The route's attributes, but for its next hop if that came with the - /// prefixes (MP_REACH_NLRI). - pub attributes: &'a [PathAttribute], - pub next_hop: Option, - pub direction: Direction, -} - -/// What a policy made of a route. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct Verdict { - /// Whether the route is taken, or announced. - pub accept: bool, - /// The route's attributes as the policy left them, if it changed any. - pub attributes: Option>, - /// Whether the policy asked what the route's prefix is. If it did not, - /// its answer is the same for any other prefix with these attributes - /// from this peer, and need not be asked for again. - pub saw_prefix: bool, -} - -/// One function of a policy. Cheap to clone. -#[derive(Debug, Clone)] -pub struct Hook { - policy: Arc, - name: String, -} - -impl Hook { - pub fn name(&self) -> &str { - &self.name - } - - /// Asks the function about a route from, or for, `peer`. - pub fn run(&self, peer: &Peer, subject: &Subject<'_>) -> Result { - let route = Route::new(subject, peer); - let returned = self - .policy - .engine - .call_fn_with_options::( - // The script's top level was run when it was loaded. - CallFnOptions::new().eval_ast(false), - &mut Scope::new(), - &self.policy.ast, - &self.name, - (route.clone(), peer.clone()), - ) - .map_err(|e| PolicyError::Run { - function: self.name.clone(), - message: e.to_string(), - })?; - // A function that ends without saying has no objection. - let accept = if returned.is_unit() { - true - } else { - returned.as_bool().map_err(|_| PolicyError::Returned { - function: self.name.clone(), - returned: returned.to_string(), - })? - }; - let (attributes, saw_prefix) = route.finish(subject.attributes); - Ok(Verdict { - accept, - attributes: attributes.filter(|_| accept), - saw_prefix, - }) - } -} diff --git a/crates/policy/src/route.rs b/crates/policy/src/route.rs deleted file mode 100644 index b65b549..0000000 --- a/crates/policy/src/route.rs +++ /dev/null @@ -1,629 +0,0 @@ -//! What a policy function is given: the route and the peer. -//! -//! # The route -//! -//! | In a script | | -//! |---|---| -//! | `route.prefix` | `"203.0.113.0/24"` | -//! | `route.prefix_len` | `24` | -//! | `route.within("203.0.112.0/22")` | whether the prefix is that one or inside it | -//! | `route.family` | `"ipv4"` or `"ipv6"` | -//! | `route.as_path` | `[64602, 3356, 15169]`, the neighbor's AS first | -//! | `route.as_path_len` | as route selection counts it: a set is one | -//! | `route.origin_as`, `route.neighbor_as` | the last and the first AS of the path; 0 if there is none | -//! | `route.path_has(3356)` | whether the AS is anywhere on the path | -//! | `route.origin` | `"igp"`, `"egp"` or `"incomplete"` | -//! | `route.next_hop` | `"192.0.2.1"` | -//! | `route.med`, `route.local_pref` | the number, or `()` if the route has none; both can be set, and unset with `()` | -//! | `route.communities` | `["3356:3", "3356:2012"]` | -//! | `route.has_community("3356:3")` | also `(3356, 3)`, and `"3356:*"` for any of that AS | -//! | `route.add_community("64601:100")` | also `(64601, 100)` | -//! | `route.remove_community("3356:*")` | all that match; `"*:*"` for all there are | -//! | `route.large_communities`, `route.has_large_community("64601:1:*")`, `route.add_large_community(..)`, `route.remove_large_community(..)` | the same for large communities (RFC 8092) | -//! | `route.prepend(2)` | on the way out: our AS that many more times on the path | -//! -//! # The peer -//! -//! `peer.name` (as configured), `peer.asn`, `peer.address`, `peer.role` -//! (`"customer"`, `"provider"`, `"peer"`, `"route_server"`, -//! `"route_server_client"`, or `""` if none is configured), `peer.ibgp`, -//! and `peer.local_asn`, which is our own. - -use std::net::IpAddr; -use std::sync::{Arc, Mutex, MutexGuard, PoisonError}; - -use bgp_packet::community::{format_large, format_standard}; -use bgp_packet::message::{ - AsPathAttribute, AsPathSegment, CommunitiesPathAttribute, LargeCommunitiesPathAttribute, - LargeCommunity, LocalPrefPathAttribute, MultiExitDiscPathAttribute, NlriNextHop, - OriginPathAttribute, PathAttribute, -}; -use bgp_primitives::constants::IpVersion; -use bgp_primitives::ip_prefix::IpPrefix; -use rhai::{Array, Dynamic, Engine, EvalAltResult, INT, ImmutableString}; - -use crate::{Direction, Subject}; - -type Fallible = Result>; - -/// A peer, as a script sees it. Made once for a session. -#[derive(Debug, Clone)] -pub struct Peer { - name: ImmutableString, - address: ImmutableString, - role: ImmutableString, - asn: u32, - local_asn: u32, - ibgp: bool, -} - -impl Peer { - /// `role` is the peer's role by the name scripts know it under, or - /// empty. - pub fn new(name: &str, address: IpAddr, asn: u32, local_asn: u32, role: &str) -> Self { - Self { - name: name.into(), - address: address.to_string().into(), - role: role.into(), - asn, - local_asn, - ibgp: asn == local_asn, - } - } -} - -/// The route a function is looking at, and what it has done to it. -#[derive(Debug)] -struct State { - prefix: IpPrefix, - direction: Direction, - local_asn: u32, - ibgp: bool, - /// The AS numbers of the path in order, sets and all. - as_path: Vec, - as_path_len: usize, - origin_as: u32, - neighbor_as: u32, - origin: OriginPathAttribute, - next_hop: Option, - communities: Vec<(u16, u16)>, - large_communities: Vec, - local_pref: Option, - med: Option, - /// How many more times our AS is to go on the path. - prepend: usize, - saw_prefix: bool, - /// Which of the above the function changed. - changed: Changed, -} - -#[derive(Debug, Default, Clone, Copy)] -struct Changed { - communities: bool, - large_communities: bool, - local_pref: bool, - med: bool, -} - -/// The handle a script holds. The function is given a copy and changes -/// what it points to; the caller reads that afterwards. -#[derive(Debug, Clone)] -pub(crate) struct Route(Arc>); - -impl Route { - pub(crate) fn new(subject: &Subject<'_>, peer: &Peer) -> Self { - let mut state = State { - prefix: *subject.prefix, - direction: subject.direction, - local_asn: peer.local_asn, - ibgp: peer.ibgp, - as_path: Vec::new(), - as_path_len: 0, - origin_as: 0, - neighbor_as: 0, - origin: OriginPathAttribute::INCOMPLETE, - next_hop: subject.next_hop, - communities: Vec::new(), - large_communities: Vec::new(), - local_pref: None, - med: None, - prepend: 0, - saw_prefix: false, - changed: Changed::default(), - }; - for attribute in subject.attributes { - match attribute { - PathAttribute::Origin(origin) => state.origin = *origin, - PathAttribute::AsPath(path) => { - state.as_path_len = path.hops(); - let first = path.segments.first().filter(|segment| segment.ordered); - let last = path.segments.last().filter(|segment| segment.ordered); - state.neighbor_as = first.and_then(|s| s.path.first()).copied().unwrap_or(0); - state.origin_as = last.and_then(|s| s.path.last()).copied().unwrap_or(0); - state.as_path = path - .segments - .iter() - .flat_map(|segment| segment.path.iter().copied()) - .collect(); - } - PathAttribute::NextHop(next_hop) => { - state.next_hop = Some(NlriNextHop::Ipv4(next_hop.0)); - } - PathAttribute::MultiExitDisc(med) => state.med = Some(med.0), - PathAttribute::LocalPref(local_pref) => state.local_pref = Some(local_pref.0), - PathAttribute::Communities(communities) => { - state.communities.clone_from(&communities.0); - } - PathAttribute::LargeCommunities(large) => { - state.large_communities.clone_from(&large.communities); - } - _ => {} - } - } - Self(Arc::new(Mutex::new(state))) - } - - fn state(&self) -> MutexGuard<'_, State> { - self.0.lock().unwrap_or_else(PoisonError::into_inner) - } - - /// What the function left: `attributes` as changed, if it changed - /// any, and whether it asked for the prefix. - pub(crate) fn finish( - &self, - attributes: &[PathAttribute], - ) -> (Option>, bool) { - let state = self.state(); - let changed = state.changed; - let prepend = state.prepend > 0 && !state.ibgp; - let any = changed.communities - || changed.large_communities - || changed.local_pref - || changed.med - || prepend; - if !any { - return (None, state.saw_prefix); - } - let mut attributes = attributes.to_vec(); - if changed.communities { - let communities = (!state.communities.is_empty()).then(|| { - PathAttribute::Communities(CommunitiesPathAttribute(state.communities.clone())) - }); - put( - &mut attributes, - bgp_packet::message::attr_type::COMMUNITIES, - communities, - ); - } - if changed.large_communities { - let large = (!state.large_communities.is_empty()).then(|| { - PathAttribute::LargeCommunities(LargeCommunitiesPathAttribute { - communities: state.large_communities.clone(), - }) - }); - put( - &mut attributes, - bgp_packet::message::attr_type::LARGE_COMMUNITIES, - large, - ); - } - if changed.local_pref { - let local_pref = state - .local_pref - .map(|value| PathAttribute::LocalPref(LocalPrefPathAttribute(value))); - put( - &mut attributes, - bgp_packet::message::attr_type::LOCAL_PREF, - local_pref, - ); - } - if changed.med { - let med = state - .med - .map(|value| PathAttribute::MultiExitDisc(MultiExitDiscPathAttribute(value))); - put( - &mut attributes, - bgp_packet::message::attr_type::MULTI_EXIT_DISC, - med, - ); - } - if prepend { - let ours = std::iter::repeat_n(state.local_asn, state.prepend); - for attribute in &mut attributes { - if let PathAttribute::AsPath(path) = attribute { - prepend_to(path, ours); - break; - } - } - } - (Some(attributes), state.saw_prefix) - } -} - -/// Puts `ours` at the front of `path`. -fn prepend_to(path: &mut AsPathAttribute, ours: impl Iterator) { - match path.segments.first_mut() { - Some(first) if first.ordered => { - first.path.splice(0..0, ours); - } - _ => path.segments.insert( - 0, - AsPathSegment { - ordered: true, - path: ours.collect(), - }, - ), - } -} - -/// Makes the attribute of type `type_code` in `attributes` be `with`: -/// replaced, added in the order of the type codes, or taken out. -fn put(attributes: &mut Vec, type_code: u8, with: Option) { - let at = attributes - .iter() - .position(|attribute| attribute.type_code() == type_code); - match (at, with) { - (Some(at), Some(with)) => attributes[at] = with, - (Some(at), None) => { - attributes.remove(at); - } - (None, Some(with)) => { - let after = attributes - .iter() - .position(|attribute| attribute.type_code() > type_code) - .unwrap_or(attributes.len()); - attributes.insert(after, with); - } - (None, None) => {} - } -} - -fn wrong(what: impl Into) -> Box { - what.into().into() -} - -/// A number a script gave, as one that fits where it is going. -fn number>(value: INT, what: &str) -> Fallible { - T::try_from(value).map_err(|_| wrong(format!("{value} is not {what}"))) -} - -/// One part of a community as a script writes it: a number, or `*` for -/// any. -fn part(text: &str, whole: &str) -> Fallible> { - if text == "*" { - return Ok(None); - } - text.parse() - .map(Some) - .map_err(|_| wrong(format!("{whole:?} is not a community"))) -} - -type Pattern = (Option, Option); -type LargePattern = (Option, Option, Option); - -fn pattern(text: &str) -> Fallible { - let (high, low) = text.split_once(':').ok_or_else(|| { - wrong(format!( - "{text:?} is not a community: it is written 64601:100" - )) - })?; - Ok((part(high, text)?, part(low, text)?)) -} - -fn large_pattern(text: &str) -> Fallible { - let mut parts = text.splitn(3, ':'); - let mut next = || { - parts.next().ok_or_else(|| { - wrong(format!( - "{text:?} is not a large community: it is written 64601:1:2" - )) - }) - }; - let (admin, first, second) = (next()?, next()?, next()?); - Ok((part(admin, text)?, part(first, text)?, part(second, text)?)) -} - -/// A pattern with nothing left open: a community to add. -fn exact((high, low): Pattern, text: &str) -> Fallible<(u16, u16)> { - high.zip(low).ok_or_else(|| { - wrong(format!( - "{text:?} cannot be added: it stands for more than one" - )) - }) -} - -fn exact_large((admin, first, second): LargePattern, text: &str) -> Fallible { - match (admin, first, second) { - (Some(global_admin), Some(data_1), Some(data_2)) => Ok(LargeCommunity { - global_admin, - data_1, - data_2, - }), - _ => Err(wrong(format!( - "{text:?} cannot be added: it stands for more than one" - ))), - } -} - -fn fits(wanted: Option, has: T) -> bool { - wanted.is_none_or(|wanted| wanted == has) -} - -fn matches((high, low): Pattern, community: (u16, u16)) -> bool { - fits(high, community.0) && fits(low, community.1) -} - -fn matches_large((admin, first, second): LargePattern, community: &LargeCommunity) -> bool { - fits(admin, community.global_admin) - && fits(first, community.data_1) - && fits(second, community.data_2) -} - -impl State { - fn add_community(&mut self, community: (u16, u16)) { - if !self.communities.contains(&community) { - self.communities.push(community); - self.changed.communities = true; - } - } - - fn remove_communities(&mut self, pattern: Pattern) { - let before = self.communities.len(); - self.communities - .retain(|community| !matches(pattern, *community)); - self.changed.communities |= self.communities.len() != before; - } - - fn set_local_pref(&mut self, value: Option) { - self.changed.local_pref |= self.local_pref != value; - self.local_pref = value; - } - - fn set_med(&mut self, value: Option) { - self.changed.med |= self.med != value; - self.med = value; - } -} - -fn optional(value: Option) -> Dynamic { - value.map_or(Dynamic::UNIT, |value| Dynamic::from(INT::from(value))) -} - -/// Tells `engine` what a route and a peer are, and what can be asked of -/// them. -pub(crate) fn register(engine: &mut Engine) { - engine.register_type_with_name::("Route"); - engine.register_type_with_name::("Peer"); - - // The peer. - engine.register_get("name", |peer: &mut Peer| peer.name.clone()); - engine.register_get("address", |peer: &mut Peer| peer.address.clone()); - engine.register_get("role", |peer: &mut Peer| peer.role.clone()); - engine.register_get("asn", |peer: &mut Peer| INT::from(peer.asn)); - engine.register_get("local_asn", |peer: &mut Peer| INT::from(peer.local_asn)); - engine.register_get("ibgp", |peer: &mut Peer| peer.ibgp); - - // The prefix. Asking for it is noted: the answer to a function that - // did not is good for every prefix. - engine.register_get("prefix", |route: &mut Route| { - let mut state = route.state(); - state.saw_prefix = true; - state.prefix.to_string() - }); - engine.register_get("prefix_len", |route: &mut Route| { - let mut state = route.state(); - state.saw_prefix = true; - INT::from(state.prefix.length()) - }); - engine.register_fn( - "within", - |route: &mut Route, around: &str| -> Fallible { - let around = IpPrefix::try_from(around) - .map_err(|e| wrong(format!("{around:?} is not a prefix: {e}")))?; - let mut state = route.state(); - state.saw_prefix = true; - Ok(around.contains(&state.prefix)) - }, - ); - // Every route of an UPDATE's one batch is of the same family. - engine.register_get("family", |route: &mut Route| { - match route.state().prefix.address_family() { - IpVersion::Ipv4 => "ipv4", - IpVersion::Ipv6 => "ipv6", - } - }); - - // The path. - engine.register_get("as_path", |route: &mut Route| -> Array { - route - .state() - .as_path - .iter() - .map(|asn| Dynamic::from(INT::from(*asn))) - .collect() - }); - engine.register_get("as_path_len", |route: &mut Route| { - route.state().as_path_len as INT - }); - engine.register_get("origin_as", |route: &mut Route| { - INT::from(route.state().origin_as) - }); - engine.register_get("neighbor_as", |route: &mut Route| { - INT::from(route.state().neighbor_as) - }); - engine.register_fn("path_has", |route: &mut Route, asn: INT| { - u32::try_from(asn).is_ok_and(|asn| route.state().as_path.contains(&asn)) - }); - engine.register_fn("prepend", |route: &mut Route, times: INT| -> Fallible<()> { - let mut state = route.state(); - if state.direction == Direction::Import { - return Err(wrong( - "prepend is for routes on their way out; to make a route less preferred on \ - its way in, set its local_pref", - )); - } - // More than this is a mistake, and makes the route no worse. - state.prepend = (0..=16) - .contains(×) - .then_some(times as usize) - .ok_or_else(|| wrong(format!("prepend({times}): from 0 to 16 times")))?; - Ok(()) - }); - - // The other attributes that are looked at. - engine.register_get("origin", |route: &mut Route| match route.state().origin { - OriginPathAttribute::IGP => "igp", - OriginPathAttribute::EGP => "egp", - OriginPathAttribute::INCOMPLETE => "incomplete", - }); - engine.register_get("next_hop", |route: &mut Route| { - route - .state() - .next_hop - .map_or_else(String::new, |next_hop| match next_hop { - NlriNextHop::Ipv4(address) => address.to_string(), - NlriNextHop::Ipv6(address) => address.to_string(), - NlriNextHop::Ipv6WithLl { global, .. } => global.to_string(), - }) - }); - - // Those that are also set. - engine.register_get("local_pref", |route: &mut Route| { - optional(route.state().local_pref) - }); - engine.register_set( - "local_pref", - |route: &mut Route, value: INT| -> Fallible<()> { - let value = number(value, "a LOCAL_PREF")?; - route.state().set_local_pref(Some(value)); - Ok(()) - }, - ); - engine.register_set("local_pref", |route: &mut Route, (): ()| { - route.state().set_local_pref(None); - }); - engine.register_get("med", |route: &mut Route| optional(route.state().med)); - engine.register_set("med", |route: &mut Route, value: INT| -> Fallible<()> { - let value = number(value, "a MULTI_EXIT_DISC")?; - route.state().set_med(Some(value)); - Ok(()) - }); - engine.register_set("med", |route: &mut Route, (): ()| { - route.state().set_med(None); - }); - - // Communities. - engine.register_get("communities", |route: &mut Route| -> Array { - route - .state() - .communities - .iter() - .map(|community| Dynamic::from(format_standard(*community))) - .collect() - }); - engine.register_fn( - "has_community", - |route: &mut Route, text: &str| -> Fallible { - let wanted = pattern(text)?; - Ok(route - .state() - .communities - .iter() - .any(|community| matches(wanted, *community))) - }, - ); - engine.register_fn( - "has_community", - |route: &mut Route, high: INT, low: INT| -> Fallible { - let wanted = ( - number(high, "half a community")?, - number(low, "half a community")?, - ); - Ok(route.state().communities.contains(&wanted)) - }, - ); - engine.register_fn( - "add_community", - |route: &mut Route, text: &str| -> Fallible<()> { - let community = exact(pattern(text)?, text)?; - route.state().add_community(community); - Ok(()) - }, - ); - engine.register_fn( - "add_community", - |route: &mut Route, high: INT, low: INT| -> Fallible<()> { - let community = ( - number(high, "half a community")?, - number(low, "half a community")?, - ); - route.state().add_community(community); - Ok(()) - }, - ); - engine.register_fn( - "remove_community", - |route: &mut Route, text: &str| -> Fallible<()> { - let unwanted = pattern(text)?; - route.state().remove_communities(unwanted); - Ok(()) - }, - ); - engine.register_fn( - "remove_community", - |route: &mut Route, high: INT, low: INT| -> Fallible<()> { - let unwanted = ( - Some(number(high, "half a community")?), - Some(number(low, "half a community")?), - ); - route.state().remove_communities(unwanted); - Ok(()) - }, - ); - - // Large communities. - engine.register_get("large_communities", |route: &mut Route| -> Array { - route - .state() - .large_communities - .iter() - .map(|community| Dynamic::from(format_large(community))) - .collect() - }); - engine.register_fn( - "has_large_community", - |route: &mut Route, text: &str| -> Fallible { - let wanted = large_pattern(text)?; - Ok(route - .state() - .large_communities - .iter() - .any(|community| matches_large(wanted, community))) - }, - ); - engine.register_fn( - "add_large_community", - |route: &mut Route, text: &str| -> Fallible<()> { - let community = exact_large(large_pattern(text)?, text)?; - let mut state = route.state(); - if !state.large_communities.contains(&community) { - state.large_communities.push(community); - state.changed.large_communities = true; - } - Ok(()) - }, - ); - engine.register_fn( - "remove_large_community", - |route: &mut Route, text: &str| -> Fallible<()> { - let unwanted = large_pattern(text)?; - let mut state = route.state(); - let before = state.large_communities.len(); - state - .large_communities - .retain(|community| !matches_large(unwanted, community)); - state.changed.large_communities |= state.large_communities.len() != before; - Ok(()) - }, - ); -} diff --git a/crates/policy/tests/scripts.rs b/crates/policy/tests/scripts.rs deleted file mode 100644 index 67ab7ad..0000000 --- a/crates/policy/tests/scripts.rs +++ /dev/null @@ -1,661 +0,0 @@ -//! Policies as an operator would write them, run on routes. - -use std::net::Ipv4Addr; -use std::sync::Arc; - -use bgp_packet::message::{ - AsPathAttribute, AsPathSegment, CommunitiesPathAttribute, LargeCommunitiesPathAttribute, - LargeCommunity, LocalPrefPathAttribute, MultiExitDiscPathAttribute, NextHopPathAttribute, - OriginPathAttribute, PathAttribute, -}; -use bgp_policy::{Direction, Peer, Policy, PolicyError, Subject, Verdict}; -use bgp_primitives::ip_prefix::IpPrefix; - -fn prefix(s: &str) -> IpPrefix { - IpPrefix::try_from(s).unwrap() -} - -fn peer(role: &str) -> Peer { - Peer::new( - "transit-a", - Ipv4Addr::new(192, 0, 2, 2).into(), - 64602, - 64601, - role, - ) -} - -/// A route by way of AS 64602 and 3356 from 15169, with communities of -/// 3356. -fn attributes() -> Vec { - vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![64602, 3356, 15169]), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 2))), - PathAttribute::Communities(CommunitiesPathAttribute(vec![(3356, 3), (3356, 2012)])), - ] -} - -fn run_on( - script: &str, - function: &str, - peer: &Peer, - route: &str, - attributes: &[PathAttribute], - direction: Direction, -) -> Result { - let policy = Policy::compile(script)?; - policy.hook(function)?.run( - peer, - &Subject { - prefix: &prefix(route), - attributes, - next_hop: None, - direction, - }, - ) -} - -fn import(script: &str, route: &str, attributes: &[PathAttribute]) -> Verdict { - run_on( - script, - "policy", - &peer("provider"), - route, - attributes, - Direction::Import, - ) - .unwrap() -} - -fn communities(attributes: &[PathAttribute]) -> Vec<(u16, u16)> { - attributes - .iter() - .find_map(|attribute| match attribute { - PathAttribute::Communities(communities) => Some(communities.0.clone()), - _ => None, - }) - .unwrap_or_default() -} - -/// The policy this was built for: mark routes by where they were learned, -/// and prefer some to others. -#[test] -fn routes_are_marked_and_preferred_by_who_sent_them() { - let script = r#" - const LEARNED_FROM = #{ customer: "64601:1000", peer: "64601:2000", provider: "64601:3000" }; - const PREFERENCE = #{ customer: 300, peer: 200, provider: 100 }; - - fn policy(route, peer) { - // What others marked their routes with means nothing here. - route.remove_community("64601:*"); - route.add_community(LEARNED_FROM[peer.role]); - route.local_pref = PREFERENCE[peer.role]; - true - } - "#; - let mut given = attributes(); - given[3] = PathAttribute::Communities(CommunitiesPathAttribute(vec![ - (3356, 3), - (64601, 1000), - (64601, 7), - ])); - for (role, mark, preference) in [ - ("customer", 1000, 300), - ("peer", 2000, 200), - ("provider", 3000, 100), - ] { - let verdict = run_on( - script, - "policy", - &peer(role), - "203.0.113.0/24", - &given, - Direction::Import, - ) - .unwrap(); - assert!(verdict.accept); - assert!(!verdict.saw_prefix, "the answer is good for any prefix"); - let changed = verdict.attributes.expect("the route was changed"); - assert_eq!(communities(&changed), [(3356, 3), (64601, mark)], "{role}"); - assert!( - changed.contains(&PathAttribute::LocalPref(LocalPrefPathAttribute( - preference - ))), - "{role}: {changed:?}" - ); - // In the order of their type codes: LOCAL_PREF (5) before - // COMMUNITIES (8). - let codes: Vec = changed.iter().map(PathAttribute::type_code).collect(); - assert_eq!(codes, [1, 2, 3, 5, 8]); - } -} - -#[test] -fn a_policy_refuses_by_what_a_route_carries() { - let script = r#" - const BLACKHOLE = "65535:666"; - fn policy(route, peer) { - if route.has_community(BLACKHOLE) { return false; } - if route.has_community("3356:*") && route.path_has(174) { return false; } - if route.as_path_len > 5 || route.origin_as == 64512 { return false; } - if route.origin != "igp" { return false; } - true - } - "#; - let verdict = import(script, "203.0.113.0/24", &attributes()); - assert_eq!( - verdict, - Verdict { - accept: true, - attributes: None, - saw_prefix: false - }, - "nothing to object to, and nothing changed" - ); - - let mut blackholed = attributes(); - blackholed[3] = PathAttribute::Communities(CommunitiesPathAttribute(vec![(65535, 666)])); - assert!(!import(script, "203.0.113.0/24", &blackholed).accept); - - let mut by_cogent = attributes(); - by_cogent[1] = AsPathAttribute::from_asns(vec![64602, 174, 15169]); - assert!(!import(script, "203.0.113.0/24", &by_cogent).accept); - - let mut long = attributes(); - long[1] = AsPathAttribute::from_asns(vec![64602; 6]); - assert!(!import(script, "203.0.113.0/24", &long).accept); - - let mut incomplete = attributes(); - incomplete[0] = PathAttribute::Origin(OriginPathAttribute::INCOMPLETE); - assert!(!import(script, "203.0.113.0/24", &incomplete).accept); -} - -/// A function that asks for the prefix is asked about every prefix; the -/// verdict says that it did. -#[test] -fn a_policy_that_looks_at_the_prefix_says_so() { - let script = r#" - fn policy(route, peer) { - if route.family == "ipv6" { return route.prefix_len <= 48; } - if route.within("203.0.112.0/22") { route.local_pref = 50; } - route.prefix != "198.51.100.0/24" - } - "#; - let inside = import(script, "203.0.113.0/24", &attributes()); - assert!(inside.accept && inside.saw_prefix); - assert!( - inside - .attributes - .unwrap() - .contains(&PathAttribute::LocalPref(LocalPrefPathAttribute(50))) - ); - let outside = import(script, "192.0.2.0/24", &attributes()); - assert_eq!( - outside, - Verdict { - accept: true, - attributes: None, - saw_prefix: true - } - ); - assert!(!import(script, "198.51.100.0/24", &attributes()).accept); - assert!(import(script, "2001:db8::/48", &attributes()).accept); - assert!(!import(script, "2001:db8::/56", &attributes()).accept); -} - -#[test] -fn everything_about_a_route_and_a_peer_can_be_read() { - let script = r#" - fn policy(route, peer) { - if route.as_path != [64602, 3356, 15169] { throw "as_path"; } - if route.as_path_len != 3 { throw "as_path_len"; } - if route.neighbor_as != 64602 || route.origin_as != 15169 { throw "ends"; } - if route.next_hop != "192.0.2.2" { throw "next_hop " + route.next_hop; } - if route.communities != ["3356:3", "3356:2012"] { throw "communities"; } - if !route.has_community(3356, 3) || route.has_community(3356, 4) { throw "has"; } - if route.large_communities != ["64601:1:2"] { throw "large"; } - if !route.has_large_community("64601:*:2") { throw "has large"; } - if route.med != 70 || route.local_pref != () { throw "numbers"; } - if peer.name != "transit-a" || peer.asn != 64602 || peer.local_asn != 64601 { - throw "peer"; - } - if peer.address != "192.0.2.2" || peer.role != "provider" || peer.ibgp { - throw "peer"; - } - } - "#; - let mut given = attributes(); - given.push(PathAttribute::MultiExitDisc(MultiExitDiscPathAttribute(70))); - given.push(PathAttribute::LargeCommunities( - LargeCommunitiesPathAttribute { - communities: vec![LargeCommunity { - global_admin: 64601, - data_1: 1, - data_2: 2, - }], - }, - )); - // It ends without returning anything: no objection. - let verdict = import(script, "203.0.113.0/24", &given); - assert!(verdict.accept && verdict.attributes.is_none()); -} - -#[test] -fn everything_that_can_be_changed_is_changed_only_if_it_differs() { - let script = r#" - fn same(route, peer) { - route.add_community("3356:3"); - route.remove_community("1:1"); - route.remove_large_community("1:*:*"); - route.local_pref = (); - route.med = (); - } - fn other(route, peer) { - route.remove_community("*:*"); - route.add_large_community("64601:1:2"); - route.add_large_community("64601:1:2"); - route.med = 5; - route.local_pref = 150; - route.local_pref = (); - } - "#; - let policy = Policy::compile(script).unwrap(); - let ask = |function: &str| { - policy - .hook(function) - .unwrap() - .run( - &peer(""), - &Subject { - prefix: &prefix("203.0.113.0/24"), - attributes: &attributes(), - next_hop: None, - direction: Direction::Import, - }, - ) - .unwrap() - }; - assert_eq!(ask("same").attributes, None); - let changed = ask("other").attributes.unwrap(); - assert_eq!( - changed, - [ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![64602, 3356, 15169]), - PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 2))), - PathAttribute::MultiExitDisc(MultiExitDiscPathAttribute(5)), - PathAttribute::LargeCommunities(LargeCommunitiesPathAttribute { - communities: vec![LargeCommunity { - global_admin: 64601, - data_1: 1, - data_2: 2 - }] - }), - ], - "no communities left, a MED and a large community added, LOCAL_PREF as it was" - ); -} - -/// On the way out a policy says what a peer is told, and how. -#[test] -fn a_route_is_announced_with_what_the_policy_gives_it() { - let script = r#" - fn to_peer(route, peer) { - if route.has_community("64601:3000") { return false; } // learned from a provider - if peer.name == "transit-a" { route.prepend(2); } - route.remove_community("64601:*"); - true - } - "#; - let ours = |communities: Vec<(u16, u16)>| { - vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![64601]), - PathAttribute::Communities(CommunitiesPathAttribute(communities)), - ] - }; - let export = |attributes: &[PathAttribute]| { - run_on( - script, - "to_peer", - &peer("peer"), - "193.0.0.0/21", - attributes, - Direction::Export, - ) - .unwrap() - }; - assert!(!export(&ours(vec![(64601, 3000)])).accept); - let verdict = export(&ours(vec![(64601, 1000), (64601, 9)])); - assert_eq!( - verdict.attributes.unwrap(), - [ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![64601, 64601, 64601]), - ] - ); - - // Prepending to a path that begins with a set, or is empty. - let set_first = vec![PathAttribute::AsPath(AsPathAttribute { - segments: vec![AsPathSegment { - ordered: false, - path: vec![1, 2], - }], - })]; - let prepended = run_on( - "fn f(route, peer) { route.prepend(1); }", - "f", - &peer(""), - "193.0.0.0/21", - &set_first, - Direction::Export, - ) - .unwrap(); - assert_eq!( - prepended.attributes.unwrap(), - [PathAttribute::AsPath(AsPathAttribute { - segments: vec![ - AsPathSegment { - ordered: true, - path: vec![64601] - }, - AsPathSegment { - ordered: false, - path: vec![1, 2] - } - ] - })] - ); -} - -/// A function that fails has refused the route: the caller is told what -/// went wrong, and has nothing to take. -#[test] -fn a_policy_that_fails_gives_no_verdict() { - let failure = |script: &str| { - run_on( - script, - "policy", - &peer("provider"), - "203.0.113.0/24", - &attributes(), - Direction::Import, - ) - .unwrap_err() - .to_string() - }; - let cases = [ - ( - "fn policy(route, peer) { route.add_community(\"3356\"); }", - "is not a community", - ), - ( - "fn policy(route, peer) { route.add_community(\"3356:*\"); }", - "stands for more than one", - ), - ( - "fn policy(route, peer) { route.add_community(70000, 1); }", - "70000 is not half a community", - ), - ( - "fn policy(route, peer) { route.local_pref = -1; }", - "-1 is not a LOCAL_PREF", - ), - ( - "fn policy(route, peer) { route.within(\"nowhere\") }", - "is not a prefix", - ), - ( - "fn policy(route, peer) { route.prepend(1); }", - "prepend is for routes on their way out", - ), - ("fn policy(route, peer) { route.nonsense }", "nonsense"), - ("fn policy(route, peer) { throw \"no\"; }", "no"), - ( - "fn policy(route, peer) { 7 }", - "returned 7; a policy returns true or false", - ), - // One that never ends is stopped. - ("fn policy(route, peer) { loop { } }", "operations"), - ( - "fn policy(route, peer) { policy(route, peer) }", - "Stack overflow", - ), - ]; - for (script, complaint) in cases { - let said = failure(script); - assert!(said.contains(complaint), "{script}: {said}"); - } -} - -#[test] -fn a_script_that_is_wrong_is_found_when_it_is_loaded() { - let load = |script: &str, function: &str| -> PolicyError { - match Policy::compile(script) { - Err(e) => e, - Ok(policy) => policy.hook(function).unwrap_err(), - } - }; - assert!(matches!( - load("fn policy(route, peer) {", "policy"), - PolicyError::Parse(_) - )); - assert!(matches!( - load("let x = 1 / 0;", "policy"), - PolicyError::Load(_) - )); - assert_eq!( - load( - "fn from_a(r, p) { true } fn from_b(r, p) { true }", - "from_c" - ) - .to_string(), - "the script has no function \"from_c\"; it has: from_a, from_b" - ); - assert_eq!( - load("fn policy(route) { true }", "policy").to_string(), - "the function \"policy\" takes 1 arguments; a policy takes two, the route and the peer" - ); - // Nothing of the machine is within reach of a script. - for out_of_reach in [ - "import \"other\" as other;", - "fn policy(route, peer) { open_file(\"/etc/passwd\") }", - "fn policy(route, peer) { let t = timestamp(); true }", - ] { - let policy = Policy::compile(out_of_reach).and_then(|policy| { - policy.hook("policy").and_then(|hook| { - hook.run( - &peer(""), - &Subject { - prefix: &prefix("203.0.113.0/24"), - attributes: &attributes(), - next_hop: None, - direction: Direction::Import, - }, - ) - }) - }); - assert!(policy.is_err(), "{out_of_reach}"); - } -} - -/// Constants are for every function of the script, also one that another -/// calls; and many sessions use one script at once. -#[test] -fn a_script_is_shared_by_every_session() { - let script = r#" - const TAGS = ["64601:1", "64601:2", "64601:3"]; - fn tag(route, n) { route.add_community(TAGS[n]); } - fn policy(route, peer) { tag(route, peer.asn % 3); true } - "#; - let policy = Policy::compile(script).unwrap(); - let hook = policy.hook("policy").unwrap(); - assert_eq!(policy.functions(), ["policy", "tag"]); - let threads: Vec<_> = (0..8u32) - .map(|n| { - let hook = hook.clone(); - std::thread::spawn(move || { - let peer = Peer::new( - "p", - Ipv4Addr::new(192, 0, 2, 9).into(), - 64700 + n, - 64601, - "", - ); - for _ in 0..2000 { - let verdict = hook - .run( - &peer, - &Subject { - prefix: &prefix("203.0.113.0/24"), - attributes: &attributes(), - next_hop: None, - direction: Direction::Import, - }, - ) - .unwrap(); - let expected = (64601, 1 + ((64700 + n) % 3) as u16); - assert_eq!( - communities(&verdict.attributes.unwrap()).last(), - Some(&expected) - ); - } - }) - }) - .collect(); - for thread in threads { - thread.join().unwrap(); - } - assert_eq!(Arc::strong_count(&policy), 2); -} - -/// The example operators start from does what its comments say. -#[test] -fn the_example_script_does_what_it_says() { - let script = include_str!("../../../testdata/policy.rhai"); - let policy = Policy::compile(script).unwrap(); - let from_peer = policy.hook("from_peer").unwrap(); - let to_peer = policy.hook("to_peer").unwrap(); - let them = |name: &str, role: &str| { - Peer::new(name, Ipv4Addr::new(192, 0, 2, 2).into(), 64602, 65000, role) - }; - let with = |communities: Vec<(u16, u16)>| { - vec![ - PathAttribute::Origin(OriginPathAttribute::IGP), - AsPathAttribute::from_asns(vec![64602]), - PathAttribute::Communities(CommunitiesPathAttribute(communities)), - ] - }; - let ask = |hook: &bgp_policy::Hook, peer: &Peer, attributes: &[PathAttribute], direction| { - hook.run( - peer, - &Subject { - prefix: &prefix("203.0.113.0/24"), - attributes, - next_hop: None, - direction, - }, - ) - .unwrap() - }; - let local_pref = |attributes: &[PathAttribute]| { - attributes.iter().find_map(|attribute| match attribute { - PathAttribute::LocalPref(local_pref) => Some(local_pref.0), - _ => None, - }) - }; - - // In: marked and ranked by who it is from, whatever it was marked with. - for (role, mark, preference) in [ - ("customer", 1000, 300), - ("peer", 2000, 200), - ("provider", 3000, 100), - ] { - let verdict = ask( - &from_peer, - &them("r", role), - &with(vec![(65000, 1000), (3356, 3)]), - Direction::Import, - ); - let changed = verdict.attributes.unwrap(); - assert_eq!(communities(&changed), [(3356, 3), (65000, mark)], "{role}"); - assert_eq!(local_pref(&changed), Some(preference), "{role}"); - } - // A peer with no role is taken, with our marks removed and no more. - let verdict = ask( - &from_peer, - &them("r", ""), - &with(vec![(65000, 7)]), - Direction::Import, - ); - assert!(verdict.accept); - assert_eq!(communities(&verdict.attributes.unwrap()), []); - // The peer's own "last resort" mark, and a blackhole request. - let verdict = ask( - &from_peer, - &them("r", "peer"), - &with(vec![(64602, 80)]), - Direction::Import, - ); - assert_eq!(local_pref(&verdict.attributes.unwrap()), Some(80)); - assert!( - !ask( - &from_peer, - &them("r", "peer"), - &with(vec![(65535, 666)]), - Direction::Import - ) - .accept - ); - - // Out: what came from above goes to customers only, without our marks. - let from_provider = with(vec![(65000, 3000), (3356, 3)]); - assert!( - !ask( - &to_peer, - &them("r", "peer"), - &from_provider, - Direction::Export - ) - .accept - ); - assert!( - !ask( - &to_peer, - &them("r", "provider"), - &from_provider, - Direction::Export - ) - .accept - ); - let verdict = ask( - &to_peer, - &them("r", "customer"), - &from_provider, - Direction::Export, - ); - assert_eq!(communities(&verdict.attributes.unwrap()), [(3356, 3)]); - assert!( - !ask( - &to_peer, - &them("r", "customer"), - &with(vec![(65535, 65281)]), - Direction::Export - ) - .accept - ); - // And the path is longer to the one it should be. - let verdict = ask( - &to_peer, - &them("backup-transit", "provider"), - &with(vec![]), - Direction::Export, - ); - assert!( - verdict - .attributes - .unwrap() - .contains(&AsPathAttribute::from_asns(vec![65000, 65000, 64602])) - ); -} diff --git a/crates/server/Cargo.toml b/crates/server/Cargo.toml index c2029b3..4e81d34 100644 --- a/crates/server/Cargo.toml +++ b/crates/server/Cargo.toml @@ -12,7 +12,6 @@ bgp-packet.workspace = true bgp-primitives.workspace = true bgp-rib.workspace = true bgp-fib.workspace = true -bgp-policy.workspace = true bgp-rpki.workspace = true bytes.workspace = true futures.workspace = true diff --git a/crates/server/proto/config.proto b/crates/server/proto/config.proto index 436d821..f845843 100644 --- a/crates/server/proto/config.proto +++ b/crates/server/proto/config.proto @@ -53,49 +53,6 @@ message BgpConfig { // that the machine forwards packets by them. Left out, the daemon only // speaks BGP and the machine's forwarding is not touched. Kernel kernel = 11; - - // The operator's own rules for routes, as a script. Left out, routes are - // judged by the checks of each peer's import and by nothing else. - PolicyScript policy = 12; -} - -// A script with the operator's own rules: which routes are taken and -// announced beyond what the general checks say, and what they are marked -// with. It is written in Rhai (https://rhai.rs) as functions that are -// given a route and the peer it comes from or goes to, may change the -// route, and return whether it is taken: -// -// const BLACKHOLE = "65535:666"; -// -// fn from_transit(route, peer) { -// if route.has_community(BLACKHOLE) { return false; } -// route.add_community("64601:3000"); // learned from a provider -// route.local_pref = 80; -// true -// } -// -// What a function can read and change is listed in bin/bgp/README.md. A -// peer says which function is for its routes (import.function and -// export.function); the two names here are for peers that say nothing. -// -// The script is read when the daemon starts, and a script that does not -// parse, or a function that is named and not there, stops it from -// starting; `bgp --check` finds both. A function that fails on a route -// (it asks for something that is not there, or does not end) has refused -// that route. -message PolicyScript { - // The file the script is in; if not absolute, beside the configuration - // file. - string path = 1; - - // Or the script itself. One of the two. - string source = 2; - - // The function for routes from peers that name none; none if empty. - string import_function = 3; - - // The function for routes to peers that name none; none if empty. - string export_function = 4; } // Installing routes in the kernel. Linux only; the daemon needs the @@ -254,23 +211,6 @@ message Peer { // that says nothing (one that does not implement RFC 9234) is taken at // this configuration's word for it. Needs role. bool require_role = 10; - - // What is announced to the peer, and how. - ExportPolicy export = 11; -} - -// What is announced to a peer. -// -// The daemon announces the routes it originates; it does not yet pass on -// what it learned from other peers. When it does, those routes will be -// given to the same function. -message ExportPolicy { - // The function of the policy script (see PolicyScript) that every route - // on its way to the peer is given. It may refuse the route, which is - // then not announced, or change its communities, its MED, and how often - // our AS is on its path. Empty for the script's export_function, or for - // none. - string function = 1; } // What a peer is to us. @@ -356,12 +296,6 @@ message ImportPolicy { // to. Not needed for a peer whose role is ROUTE_SERVER, whose routes // have its clients as next hops. bool accept_third_party_next_hop = 14; - - // The function of the policy script (see PolicyScript) that every route - // from the peer is given, after it has passed all of the above. It may - // refuse the route, or change its communities, its LOCAL_PREF and its - // MED. Empty for the script's import_function, or for none. - string function = 15; } // A prefix, and optionally the more specific prefixes inside it. diff --git a/crates/server/src/advertise.rs b/crates/server/src/advertise.rs index 36b001e..9a9a99d 100644 --- a/crates/server/src/advertise.rs +++ b/crates/server/src/advertise.rs @@ -24,7 +24,6 @@ use bgp_packet::message::{ LocalPrefPathAttribute, Message, MpReachNlriPathAttribute, MpUnreachNlriPathAttribute, MultiExitDiscPathAttribute, NextHopPathAttribute, NlriNextHop, PathAttribute, UpdateMessage, }; -use bgp_policy::{Direction, Hook, PolicyError, Subject}; use bgp_primitives::constants::{AfiSafi, IpVersion}; use bgp_primitives::ip_prefix::{IpPrefix, Nlri}; use bgp_rib::table::DEFAULT_LOCAL_PREF; @@ -35,9 +34,7 @@ use tokio::sync::broadcast::error::RecvError; use tokio::task::JoinHandle; use tracing::{debug, info, warn}; -use crate::metrics::POLICY_RUNS; use crate::policy::PeerRole; -use crate::routes::policy_failed; use crate::session::{PeerId, SessionClosed, SessionEvent, SessionHandle, SessionRegistry}; /// Starts the advertiser. It hears of every session that comes up after @@ -238,7 +235,7 @@ impl Exporter { role: session.role, extended_next_hop: session.extended_next_hop, }; - let Some(mut update) = build_announce(route, &towards) else { + let Some(update) = build_announce(route, &towards) else { debug!( peer = %session.peer, prefix = %route.prefix, @@ -246,28 +243,6 @@ impl Exporter { ); return Ok(()); }; - // The operator's own rules for what this peer is told. - if let Some(export) = &session.export { - let (hook, peer) = &**export; - POLICY_RUNS.with_label_values(&["export"]).inc(); - match apply_export(&mut update, hook, peer, &route.prefix, &towards) { - Ok(true) => {} - Ok(false) => { - debug!( - peer = %session.peer, - prefix = %route.prefix, - function = hook.name(), - "Not announcing route: the policy function refused it" - ); - return Ok(()); - } - // A function that fails has refused the route. - Err(e) => { - policy_failed(hook.name(), session.name.as_deref().unwrap_or(""), &e); - return Ok(()); - } - } - } debug!(peer = %session.peer, prefix = %route.prefix, "Announcing local route"); session.send(Message::Update(update)).await?; self.announced.insert(route.prefix); @@ -290,59 +265,6 @@ impl Exporter { } } -/// Asks the policy function `hook` about the route for `prefix` that -/// `update` announces, and makes `update` what the function left of it. -/// Returns whether the route is to be announced at all. -fn apply_export( - update: &mut UpdateMessage, - hook: &Hook, - peer: &bgp_policy::Peer, - prefix: &IpPrefix, - towards: &Towards, -) -> Result { - let next_hop = update - .path_attributes - .iter() - .find_map(|attribute| match attribute { - PathAttribute::MpReachNlri(mp) => Some(mp.next_hop), - _ => None, - }); - let verdict = hook.run( - peer, - &Subject { - prefix, - attributes: &update.path_attributes, - next_hop, - direction: Direction::Export, - }, - )?; - if !verdict.accept { - return Ok(false); - } - let Some(mut attributes) = verdict.attributes else { - return Ok(true); - }; - // What the function may not decide. LOCAL_PREF does not leave the AS - // (RFC 4271 §5.1.5); and a path it made longer is still to reach a - // peer with two-octet AS numbers whole (RFC 6793 §4.2.2). - attributes.retain(|attribute| match attribute { - PathAttribute::LocalPref(_) => towards.ibgp, - PathAttribute::As4Path(_) => false, - _ => true, - }); - if !towards.four_octet_asn { - let as4_path = attributes.iter().find_map(|attribute| match attribute { - PathAttribute::AsPath(path) => path.as4_path(), - _ => None, - }); - attributes.extend(as4_path); - } - // In ascending order of type code, as they were. - attributes.sort_by_key(PathAttribute::type_code); - update.path_attributes = attributes; - Ok(true) -} - /// What about a session decides how a route is announced on it. #[derive(Debug, Clone, Copy)] pub struct Towards { diff --git a/crates/server/src/config.rs b/crates/server/src/config.rs index 0b9c75e..551eefb 100644 --- a/crates/server/src/config.rs +++ b/crates/server/src/config.rs @@ -1,5 +1,4 @@ use std::net::{IpAddr, Ipv4Addr, SocketAddr}; -use std::path::Path; use std::str::FromStr; use std::sync::Arc; use std::time::Duration; @@ -8,7 +7,6 @@ use bgp_fib::FibConfig; use bgp_packet::community; use bgp_packet::message::OriginPathAttribute; use bgp_packet::open::AddPathSendRecv; -use bgp_policy::{Hook, Policy}; use bgp_primitives::constants::{AfiSafi, IpVersion}; use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRoute, LocalRouteAttrs}; @@ -54,8 +52,6 @@ pub struct Config { /// Where route origin validation gets its data; `None` for no /// validation. pub rpki: Option, - /// The policy script, if there is one. - pub policy: Option>, } #[derive(Debug, Clone)] @@ -80,10 +76,6 @@ pub struct PeerConfig { /// Refuse the peer if it does not say what it is itself (RFC 9234 /// §4.2, "strict mode"). pub require_role: bool, - /// The operator's own rules for the routes the peer sends, and for - /// those it is sent: functions of the policy script. - pub import_hook: Option, - pub export_hook: Option, } impl Config { @@ -92,45 +84,7 @@ impl Config { self.peers.iter().find(|peer| peer.endpoint.is_at(address)) } - /// Reads a configuration whose files, if it names any, are beside it - /// in the working directory. pub fn from_proto(proto: BgpConfig) -> Result { - Self::from_proto_in(proto, Path::new(".")) - } - - /// Reads a configuration that came from a file in `directory`: other - /// files it names by a relative path are looked for there. - pub fn from_proto_in(proto: BgpConfig, directory: &Path) -> Result { - let policy = proto - .policy - .as_ref() - .map(|policy| load_policy(policy, directory)) - .transpose()?; - let default_hooks = proto.policy.as_ref().map_or(("", ""), |policy| { - ( - policy.import_function.as_str(), - policy.export_function.as_str(), - ) - }); - // The function `named`, or the one for peers that name none. - let hook = |named: &str, default: &str| -> Result, PeerProblem> { - let name = if named.is_empty() { default } else { named }; - match (&policy, name) { - (_, "") => Ok(None), - (Some(policy), name) => Ok(Some(policy.hook(name)?)), - (None, name) => Err(PeerProblem::NoPolicyScript(name.to_string())), - } - }; - // A function named for peers that name none has to be there - // whether or not there is such a peer. - if let Some(policy) = &policy { - for default in [default_hooks.0, default_hooks.1] { - if !default.is_empty() { - policy.hook(default)?; - } - } - } - // Peers refuse an identifier of 0.0.0.0, as we do theirs. let router_id = Ipv4Addr::from_str(&proto.router_id) .ok() @@ -164,10 +118,7 @@ impl Config { peer: name.clone(), problem, }; - let functions = (peer.import.function.clone(), peer.export.function.clone()); - let mut peer = parse_peer(peer).map_err(rejected)?; - peer.import_hook = hook(&functions.0, default_hooks.0).map_err(rejected)?; - peer.export_hook = hook(&functions.1, default_hooks.1).map_err(rejected)?; + let peer = parse_peer(peer).map_err(rejected)?; // RFC 9234 is about what networks are to each other. if peer.role.is_some() && peer.remote_asn == proto.local_asn { return Err(rejected(PeerProblem::RoleInOwnAs)); @@ -243,7 +194,6 @@ impl Config { http_endpoint, kernel: proto.kernel.as_ref().map(parse_kernel), rpki, - policy, }) } } @@ -349,29 +299,9 @@ fn parse_peer(peer: Peer) -> Result { import, role, require_role: peer.require_role, - import_hook: None, - export_hook: None, }) } -/// Reads the policy script, from the configuration itself or from the -/// file it names. -fn load_policy(policy: &proto::PolicyScript, directory: &Path) -> Result, ConfigError> { - let source = match (policy.path.as_str(), policy.source.as_str()) { - ("", "") => return Err(ConfigError::PolicySource), - ("", source) => source.to_string(), - (path, "") => { - let path = directory.join(path); - std::fs::read_to_string(&path).map_err(|e| ConfigError::PolicyFile { - path: path.display().to_string(), - error: e.to_string(), - })? - } - (_, _) => return Err(ConfigError::PolicySource), - }; - Ok(Policy::compile(&source)?) -} - fn parse_rpki(rpki: &proto::Rpki) -> Result { if rpki.caches.is_empty() { return Err(ConfigError::RpkiNoCaches); @@ -532,94 +462,6 @@ mod tests { parse(text).err().expect(text) } - /// Parses a configuration whose files are in `testdata`. - fn parse_in_testdata(text: &str) -> Result { - let proto = protobuf::text_format::parse_from_str::(text) - .expect("the test's textproto is well formed"); - let testdata = Path::new(env!("CARGO_MANIFEST_DIR")).join("../../testdata"); - Config::from_proto_in(proto, &testdata) - } - - /// The operator's rules are a script: in a file beside the - /// configuration or in the configuration itself, with a function for - /// each direction of each peer, or one for all that name none. - #[test] - fn the_policy_script_is_read_and_its_functions_found() { - const SCRIPT: &str = "fn a(route, peer) { true } fn b(route, peer) { false }"; - let with = |policy: &str, peers: &str| { - parse(&format!( - "{BASE}policy {{ source: \"{SCRIPT}\" {policy} }}\n\ - peers {{ name: \"r1\" endpoint {{ address: \"192.0.2.2\" }} remote_asn: 1 {peers} }}\n\ - peers {{ name: \"r2\" endpoint {{ address: \"192.0.2.3\" }} remote_asn: 2 }}" - )) - }; - let names = |config: &Config| -> Vec<(Option, Option)> { - let name = |hook: &Option| hook.as_ref().map(|hook| hook.name().to_string()); - config - .peers - .iter() - .map(|peer| (name(&peer.import_hook), name(&peer.export_hook))) - .collect() - }; - // Nobody names a function, and there is none for those: none run. - let config = with("", "").unwrap(); - assert_eq!(config.policy.as_ref().unwrap().functions(), ["a", "b"]); - assert_eq!(names(&config), [(None, None), (None, None)]); - // One peer names its own; the other gets the one for the rest. - let config = with( - "import_function: \"a\"", - "import { function: \"b\" } export { function: \"a\" }", - ) - .unwrap(); - let (a, b) = (Some("a".to_string()), Some("b".to_string())); - assert_eq!(names(&config), [(b, a.clone()), (a, None)]); - - // A function that is not there is found now, not when a route comes. - for (policy, peers) in [ - ("", "import { function: \"c\" }"), - ("", "export { function: \"c\" }"), - ("export_function: \"c\"", ""), - ] { - let said = with(policy, peers).err().expect(peers).to_string(); - assert!( - said.contains("the script has no function \"c\"; it has: a, b"), - "{said}" - ); - } - // So is a function named without a script, and a script that is - // not one. - let said = rejected(&format!( - "{BASE}peers {{ name: \"r1\" endpoint {{ address: \"192.0.2.2\" }} remote_asn: 1 \ - import {{ function: \"a\" }} }}" - )) - .to_string(); - assert_eq!( - said, - "peer \"r1\": names the policy function \"a\", and there is no policy script" - ); - let said = rejected(&format!("{BASE}policy {{ source: \"fn a(route {{\" }}")).to_string(); - assert!( - said.starts_with("policy: the script does not parse"), - "{said}" - ); - for neither_or_both in ["policy {}", "policy { path: \"a\" source: \"b\" }"] { - assert_eq!( - rejected(&format!("{BASE}{neither_or_both}")), - ConfigError::PolicySource - ); - } - - // From a file beside the configuration. - let config = - parse_in_testdata(&format!("{BASE}policy {{ path: \"policy.rhai\" }}")).unwrap(); - assert!(!config.policy.unwrap().functions().is_empty()); - let said = parse_in_testdata(&format!("{BASE}policy {{ path: \"nowhere.rhai\" }}")) - .err() - .unwrap() - .to_string(); - assert!(said.starts_with("policy: cannot read "), "{said}"); - } - /// The example that the documentation points to, and that operators /// start from: as it is, and with what it shows commented out put in. #[test] @@ -651,18 +493,7 @@ mod tests { }) .collect(); assert_ne!(shown, example, "nothing was taken for configuration"); - let config = parse_in_testdata(&shown).unwrap_or_else(|e| panic!("{e}\n{shown}")); - // The script it names is the example script, and the functions it - // names are in it. - assert!(config.policy.is_some(), "{shown}"); - assert_eq!( - config.peers[0].import_hook.as_ref().map(Hook::name), - Some("from_peer") - ); - assert_eq!( - config.peers[0].export_hook.as_ref().map(Hook::name), - Some("to_peer") - ); + let config = parse(&shown).unwrap_or_else(|e| panic!("{e}\n{shown}")); assert!(config.rpki.is_some(), "{shown}"); assert!(config.kernel.is_some(), "{shown}"); assert_eq!(config.peers[0].role, Some(PeerRole::Peer)); @@ -851,17 +682,17 @@ mod tests { other => panic!("{other}"), }; assert_eq!( - problem(peer("r1", "192.0.2.2") + peer("r1", "192.0.2.3").as_str()), + problem(peer("r1", "192.0.2.2") + &peer("r1", "192.0.2.3")), PeerProblem::DuplicateName ); assert_eq!( - problem(peer("r1", "192.0.2.2") + peer("r2", "192.0.2.2").as_str()), + problem(peer("r1", "192.0.2.2") + &peer("r2", "192.0.2.2")), PeerProblem::DuplicateAddress ); // A link-local address is one peer on each interface it is on. assert_eq!( - problem(peer("r1", "fe80::1%eth0") + peer("r2", "fe80::1%eth0").as_str()), + problem(peer("r1", "fe80::1%eth0") + &peer("r2", "fe80::1%eth0")), PeerProblem::DuplicateAddress ); let config = parse(&format!( diff --git a/crates/server/src/connection.rs b/crates/server/src/connection.rs index 31b6be8..665bb47 100644 --- a/crates/server/src/connection.rs +++ b/crates/server/src/connection.rs @@ -21,7 +21,6 @@ use bgp_packet::message::{ }; use bgp_packet::open::AddPathSendRecv; use bgp_packet::update_decode::{UpdateError, merge_as4_path}; -use bgp_policy::Hook; use bgp_primitives::ip_prefix::Nlri; use bgp_primitives::parser::ParserContext; use bgp_rib::table::{Outcome, PeerIndex, PeerInfo, RibTable}; @@ -108,10 +107,6 @@ pub struct PeerSettings { pub role: Option, /// Refuse the peer if it does not say what it is (RFC 9234). pub require_role: bool, - /// The functions of the policy script for the peer's routes and for - /// the routes it is sent. - pub import_hook: Option, - pub export_hook: Option, } impl PeerSettings { @@ -128,8 +123,6 @@ impl PeerSettings { import: Arc::new(ImportPolicy::permissive()), role: None, require_role: false, - import_hook: None, - export_hook: None, } } } @@ -145,8 +138,6 @@ impl From<&PeerConfig> for PeerSettings { import: Arc::new(peer.import.clone()), role: peer.role, require_role: peer.require_role, - import_hook: peer.import_hook.clone(), - export_hook: peer.export_hook.clone(), } } } @@ -242,9 +233,6 @@ pub async fn run_session( /// What exists only once a session is Established. struct Established { - /// The policy function for the peer's routes, and the peer as it is - /// shown to it. - import_script: Option<(Hook, bgp_policy::Peer)>, /// When it was. since: Instant, peer_id: PeerId, @@ -290,10 +278,6 @@ impl Established { .as_mut() .map_or(Checks::NONE, |validation| validation.validator.checks()), refusals, - script: self - .import_script - .as_ref() - .map(|(hook, peer)| routes::Script { hook, peer }), }; (import, &mut self.held) } @@ -462,8 +446,6 @@ struct Session { peer_name: Option, /// Which of the peer's routes are taken. import: Arc, - /// The policy script's functions for routes from and to the peer. - hooks: (Option, Option), /// What the peer is to us, if that is known. role: Option, /// Where what is refused is noted. @@ -520,7 +502,6 @@ impl Session { server, peer_name: peer.name, import: peer.import, - hooks: (peer.import_hook, peer.export_hook), role: peer.role, said: Said::new(), }; @@ -825,23 +806,7 @@ impl Session { changes: handle.validate(subject), } }); - // The peer as the policy script sees it. - let script_peer = || { - bgp_policy::Peer::new( - self.refusals.label(), - self.peer_addr.ip(), - negotiated.peer_asn, - self.server.local_asn, - self.role.map_or("", PeerRole::in_scripts), - ) - }; - handle.export = self - .hooks - .1 - .clone() - .map(|hook| Arc::new((hook, script_peer()))); self.established = Some(Established { - import_script: self.hooks.0.clone().map(|hook| (hook, script_peer())), since: Instant::now(), validation, held: Held::default(), @@ -1334,7 +1299,6 @@ mod tests { std::net::IpAddr::from(Ipv4Addr::new(192, 0, 2, 1)).into(), "test", ), - script: None, }; routes::apply(&table, &import, &mut Held::default(), &update, None).unwrap(); assert_eq!(table.len(), 1); diff --git a/crates/server/src/error.rs b/crates/server/src/error.rs index 836bacf..60dd50d 100644 --- a/crates/server/src/error.rs +++ b/crates/server/src/error.rs @@ -176,12 +176,6 @@ pub enum ConfigError { HttpPort, #[error("rpki: no caches are listed")] RpkiNoCaches, - #[error("policy: give the script's path or its source, and not both")] - PolicySource, - #[error("policy: cannot read {path}: {error}")] - PolicyFile { path: String, error: String }, - #[error("policy: {0}")] - Policy(#[from] bgp_policy::PolicyError), #[error("rpki: the port of cache {0:?} is required")] RpkiCachePort(String), #[error("peer {peer:?}: {problem}")] @@ -198,10 +192,6 @@ pub enum ConfigError { pub enum PeerProblem { #[error("endpoint is missing")] MissingEndpoint, - #[error("{0}")] - Policy(#[from] bgp_policy::PolicyError), - #[error("names the policy function {0:?}, and there is no policy script")] - NoPolicyScript(String), #[error("endpoint is invalid: {0}")] Endpoint(Box), #[error("remote_asn must be non-zero")] diff --git a/crates/server/src/metrics.rs b/crates/server/src/metrics.rs index 467a9ea..b19c6ad 100644 --- a/crates/server/src/metrics.rs +++ b/crates/server/src/metrics.rs @@ -164,24 +164,6 @@ pub static RIB_JOURNAL_ENTRIES: LazyLock = LazyLock::new(|| { .expect("metric registration") }); -pub static POLICY_RUNS: LazyLock = LazyLock::new(|| { - register_int_counter_vec!( - "bgp_policy_runs_total", - "Times a function of the policy script was asked about a route, by direction", - &["direction"] - ) - .expect("metric registration") -}); - -pub static POLICY_FAILURES: LazyLock = LazyLock::new(|| { - register_int_counter_vec!( - "bgp_policy_failures_total", - "Times a function of the policy script failed, which refuses the route, by function", - &["function"] - ) - .expect("metric registration") -}); - /// Whether each configured peer's session is up: what to alert on when a /// peer goes away, which a count of sessions cannot say. pub static PEER_UP: LazyLock = LazyLock::new(|| { @@ -225,8 +207,6 @@ pub fn init() { LazyLock::force(&RIB_PREFIXES); LazyLock::force(&RIB_ATTRIBUTE_SETS); LazyLock::force(&RIB_JOURNAL_ENTRIES); - LazyLock::force(&POLICY_RUNS); - LazyLock::force(&POLICY_FAILURES); LazyLock::force(&PEER_UP); BUILD_INFO .with_label_values(&[env!("CARGO_PKG_VERSION")]) diff --git a/crates/server/src/policy.rs b/crates/server/src/policy.rs index 5878cec..4cd5c04 100644 --- a/crates/server/src/policy.rs +++ b/crates/server/src/policy.rs @@ -188,11 +188,6 @@ pub enum Rejection { /// The next hop cannot be reached from this machine as its interfaces /// are now, so the kernel could not be given the route. NextHopUnreachable, - /// The operator's policy function for the peer refused the route. - Policy, - /// The operator's policy function failed on the route, which is as - /// good as refusing it. - PolicyFailed, } impl Rejection { @@ -206,7 +201,7 @@ impl Rejection { ) } - pub const ALL: [Self; 19] = [ + pub const ALL: [Self; 17] = [ Self::BogonPrefix, Self::TooShort, Self::TooSpecific, @@ -224,8 +219,6 @@ impl Rejection { Self::NextHopNotPeer, Self::OwnPrefix, Self::NextHopUnreachable, - Self::Policy, - Self::PolicyFailed, ]; /// Where the reason is in [`Rejection::ALL`]. @@ -252,8 +245,6 @@ impl Rejection { Self::NextHopNotPeer => "next hop is not the peer", Self::OwnPrefix => "prefix is our own", Self::NextHopUnreachable => "next hop unreachable", - Self::Policy => "refused by policy function", - Self::PolicyFailed => "policy function failed", } } } @@ -338,17 +329,6 @@ impl PeerRole { Self::RouteServerClient => "route server client", } } - - /// The role as a policy script has it in `peer.role`: one word. - pub fn in_scripts(self) -> &'static str { - match self { - Self::Provider => "provider", - Self::Customer => "customer", - Self::Peer => "peer", - Self::RouteServer => "route_server", - Self::RouteServerClient => "route_server_client", - } - } } /// Whose routes are being judged. diff --git a/crates/server/src/reach.rs b/crates/server/src/reach.rs index 6d2c14a..3ce757b 100644 --- a/crates/server/src/reach.rs +++ b/crates/server/src/reach.rs @@ -322,7 +322,6 @@ mod tests { ..Checks::NONE }, refusals: &self.refusals.of_peer(address.into(), "test"), - script: None, }; work(&self.table, &import, &mut peer.held) } diff --git a/crates/server/src/routes.rs b/crates/server/src/routes.rs index 00d3a44..86b7bb7 100644 --- a/crates/server/src/routes.rs +++ b/crates/server/src/routes.rs @@ -4,24 +4,18 @@ use std::borrow::Cow; use std::cell::OnceCell; use std::collections::HashMap; -use std::sync::{Arc, Mutex}; -use std::time::Duration; +use std::sync::Arc; use bgp_packet::message::{NlriNextHop, PathAttribute, UpdateMessage, only_to_customer_wire}; -use bgp_policy::{Direction, Hook, PolicyError, Subject}; use bgp_primitives::constants::IpVersion; use bgp_primitives::ip_prefix::Nlri; use bgp_primitives::parser::EncodeError; use bgp_rib::table::{AttrSet, Outcome, PeerIndex, RibTable}; use bgp_rpki::{PathValidity, Validity}; -use tracing::warn; -use crate::metrics::{POLICY_FAILURES, POLICY_RUNS}; use crate::policy::{self, ImportPolicy, Rejection, RouteSource}; use crate::refusals::PeerRefusals; use crate::rpki::Checks; -use crate::sync::IgnorePoison; -use crate::throttle::Throttle; /// The most routes of one peer that are held aside as invalid. Far more /// than a peer sending the whole Internet has; a bound all the same. @@ -47,30 +41,6 @@ pub(crate) struct Import<'a> { pub(crate) checks: Checks<'a>, /// Where what is refused is noted. pub(crate) refusals: &'a PeerRefusals, - /// The operator's own rules for the peer's routes, if there are any. - pub(crate) script: Option>, -} - -/// A function of the policy script, and the peer as it is shown to it. -#[derive(Clone, Copy)] -pub(crate) struct Script<'a> { - pub(crate) hook: &'a Hook, - pub(crate) peer: &'a bgp_policy::Peer, -} - -/// Says that a policy function failed. Counted every time, said once in a -/// while: a function that fails on one route fails on the next. -pub(crate) fn policy_failed(function: &str, peer: &str, error: &PolicyError) { - static SAID: Mutex = Mutex::new(Throttle::every(Duration::from_secs(60))); - POLICY_FAILURES.with_label_values(&[function]).inc(); - if let Some(unsaid) = SAID.locked().pass() { - warn!( - peer, - err = %error, - others_since_last_report = unsaid, - "A policy function failed; the route it was asked about is refused" - ); - } } /// A peer's routes that are not in the routing table because the RPKI's @@ -192,10 +162,6 @@ fn only_to_customer_mark(import: &Import<'_>, update: &UpdateMessage) -> Option< }) } -/// Routes a policy function took, with their attributes as the function -/// left them if it changed any. -type Taken<'n> = (Cow<'n, [Nlri]>, Option>); - /// One UPDATE on its way into the table. struct Applying<'a> { table: &'a RibTable, @@ -264,99 +230,6 @@ impl Applying<'_> { .checks .next_hops .is_none_or(|next_hops| next_hops.next_hop_is_near(family, &used_next_hop)); - // The operator's own rules come after the general ones, and before - // what may yet change: a route they refuse is refused for good. - for (routes, changed) in self.scripted(nlri, next_hop) { - self.admit(&routes, next_hop, origin, near, changed.as_deref())?; - } - Ok(()) - } - - /// Asks the peer's policy function, if it has one, about the routes of - /// `nlri`. Returns those it takes, with their attributes as the - /// function left them if it changed any; the others are refused here. - /// - /// The function is asked once if it does not ask for the prefix: its - /// answer is then the same for all of them. Otherwise it is asked - /// about each, and routes it treats alike are returned together. - fn scripted<'n>(&mut self, nlri: &'n [Nlri], next_hop: Option<&NlriNextHop>) -> Vec> { - let Some(script) = self.import.script else { - return vec![(Cow::Borrowed(nlri), None)]; - }; - // Without the attributes that hold prefixes, which are not the - // route's and can be long. - let names_prefixes = |attribute: &PathAttribute| { - matches!( - attribute, - PathAttribute::MpReachNlri(_) | PathAttribute::MpUnreachNlri(_) - ) - }; - let attributes: Cow<'_, [PathAttribute]> = - if self.update.path_attributes.iter().any(names_prefixes) { - let mut plain = self.update.path_attributes.clone(); - plain.retain(|attribute| !names_prefixes(attribute)); - Cow::Owned(plain) - } else { - Cow::Borrowed(&self.update.path_attributes) - }; - let mut taken: Vec> = Vec::new(); - for (at, route) in nlri.iter().enumerate() { - POLICY_RUNS.with_label_values(&["import"]).inc(); - let verdict = script.hook.run( - script.peer, - &Subject { - prefix: &route.prefix, - attributes: &attributes, - next_hop: next_hop.copied(), - direction: Direction::Import, - }, - ); - let verdict = match verdict { - Ok(verdict) => verdict, - Err(e) => { - policy_failed(script.hook.name(), self.import.refusals.label(), &e); - self.refuse(std::slice::from_ref(route), Rejection::PolicyFailed); - continue; - } - }; - // The first answer is everyone's, unless it was about the - // prefix. - let (routes, all) = if at == 0 && !verdict.saw_prefix { - (nlri, true) - } else { - (std::slice::from_ref(route), false) - }; - if !verdict.accept { - self.refuse(routes, Rejection::Policy); - } else if all { - taken.push((Cow::Borrowed(routes), verdict.attributes)); - } else { - match taken - .iter_mut() - .find(|(_, changed)| *changed == verdict.attributes) - { - Some((alike, _)) => alike.to_mut().push(*route), - None => taken.push((Cow::Owned(vec![*route]), verdict.attributes)), - } - } - if all { - break; - } - } - taken - } - - /// Puts into the table those of `nlri` that pass what is asked of - /// each route by itself. `changed` is their attributes, if the policy - /// function changed them. - fn admit( - &mut self, - nlri: &[Nlri], - next_hop: Option<&NlriNextHop>, - origin: Option, - near: bool, - changed: Option<&[PathAttribute]>, - ) -> Result<(), EncodeError> { // Nearly always every route is taken, and then nothing is copied. let mut accepted = Cow::Borrowed(nlri); let mut invalid = Vec::new(); @@ -381,7 +254,7 @@ impl Applying<'_> { if accepted.is_empty() && invalid.is_empty() { return Ok(()); } - let attrs = self.attributes(next_hop, changed)?; + let attrs = self.attributes(next_hop)?; for route in invalid { self.held.hold(route, &attrs); } @@ -459,23 +332,9 @@ impl Applying<'_> { self.applied.outcome += self.table.withdraw(self.import.peer, refused); } - /// The attribute set the UPDATE's routes go into the table with: - /// `changed`, if the policy function changed the attributes, or else - /// those the UPDATE came with. - fn attributes( - &self, - next_hop: Option<&NlriNextHop>, - changed: Option<&[PathAttribute]>, - ) -> Result, EncodeError> { + /// The attribute set the UPDATE's routes go into the table with. + fn attributes(&self, next_hop: Option<&NlriNextHop>) -> Result, EncodeError> { let store = self.table.attrs(); - if let Some(changed) = changed { - let Some(marked_by) = self.mark else { - return store.intern(changed, next_hop); - }; - let mut marked = changed.to_vec(); - marked.push(PathAttribute::OnlyToCustomer(marked_by)); - return store.intern(&marked, next_hop); - } let Some(marked_by) = self.mark else { if let Some(set) = self .attributes_wire diff --git a/crates/server/src/server.rs b/crates/server/src/server.rs index 51c7096..a00c875 100644 --- a/crates/server/src/server.rs +++ b/crates/server/src/server.rs @@ -121,8 +121,6 @@ impl BgpServer { endpoint: p.endpoint.clone(), remote_asn: p.remote_asn, role: p.role, - import_function: p.import_hook.as_ref().map(|hook| hook.name().into()), - export_function: p.export_hook.as_ref().map(|hook| hook.name().into()), }) .collect(); // The client starts now: with luck the data is there before the diff --git a/crates/server/src/session.rs b/crates/server/src/session.rs index 2e0260e..8ec2573 100644 --- a/crates/server/src/session.rs +++ b/crates/server/src/session.rs @@ -65,10 +65,6 @@ pub struct ConfiguredPeer { pub remote_asn: u32, /// What the peer is to us, if that was said. pub role: Option, - /// The functions of the policy script for routes from and to the - /// peer, by name. - pub import_function: Option, - pub export_function: Option, } /// Counts kept by a session as it goes, for whoever wants to look. @@ -149,9 +145,6 @@ pub struct SessionHandle { pub rib_peer: PeerIndex, /// How many routes the peer has, and how many were refused. pub counters: Arc, - /// The policy function for routes on their way to the peer, with the - /// peer as it is shown to it. - pub export: Option>, outbound: mpsc::Sender, /// Which of the RPKI's checks the peer's routes are subject to, if /// any, and where the session is told to look at its routes again. @@ -188,7 +181,6 @@ impl SessionHandle { add_path_send: negotiated.add_path_send.clone(), rib_peer, counters: Arc::default(), - export: None, outbound, validation: None, } diff --git a/crates/web/src/api.rs b/crates/web/src/api.rs index 66754fb..8c61abf 100644 --- a/crates/web/src/api.rs +++ b/crates/web/src/api.rs @@ -89,10 +89,6 @@ pub struct PeerView { /// Whether the peer's OPEN said the same (RFC 9234). `false` for a /// peer that does not say; one that says otherwise gets no session. pub role_confirmed: Option, - /// The functions of the policy script that the peer's routes, and - /// the routes it is sent, are given to; absent where there is none. - pub import_function: Option, - pub export_function: Option, /// Why the last connection attempt or session ended, while in Backoff. pub last_error: Option, pub retry_at_unix: Option, @@ -119,8 +115,6 @@ impl From<&SessionHandle> for PeerView { held_routes: Some(s.counters.held()), role: s.role.map(PeerRole::as_str), role_confirmed: s.role.map(|_| s.role_confirmed), - import_function: None, - export_function: None, last_error: None, retry_at_unix: None, } @@ -155,8 +149,6 @@ impl PeerView { held_routes: None, role: peer.role.map(PeerRole::as_str), role_confirmed: None, - import_function: peer.import_function.clone(), - export_function: peer.export_function.clone(), last_error, retry_at_unix, } @@ -180,8 +172,6 @@ pub fn peer_views(registry: &SessionRegistry) -> Vec { claimed.insert(session.peer); PeerView { name: Some(peer.name.clone()), - import_function: peer.import_function.clone(), - export_function: peer.export_function.clone(), ..PeerView::from(session) } } diff --git a/crates/web/static/statusz.html b/crates/web/static/statusz.html index 3f2ecb1..883901f 100644 --- a/crates/web/static/statusz.html +++ b/crates/web/static/statusz.html @@ -123,10 +123,6 @@ from const esc = s => String(s).replace(/[&<>"']/g, c => ({'&':'&','<':'<','>':'>','"':'"',"'":'''}[c])); const state = { status: null, peers: [], local: [], received: [], receivedPage: {}, refused: [], rpki: null, kernel: null }; const DASH = '-'; - // The functions of the policy script a peer's routes go through. - const policy = p => (p.import_function || p.export_function) - ? ` ${[p.import_function && 'in: ' + esc(p.import_function), p.export_function && 'out: ' + esc(p.export_function)].filter(Boolean).join(', ')}` - : ''; // ---- helpers ----------------------------------------------------------- const fmtDur = s => { @@ -186,7 +182,7 @@ from const up = p.state === 'Established'; const detail = p.state === 'Backoff' ? `${esc(p.last_error || '')}; retry in ${until(p.retry_at_unix)}` : ''; return ` - ${p.name ? esc(p.name) : '(inbound)'}${policy(p)}${esc(p.peer_addr)}${p.peer_asn}${p.role ? ` ${esc(p.role)}${p.role_confirmed ? ' ✓' : ''}` : ''} + ${p.name ? esc(p.name) : '(inbound)'}${esc(p.peer_addr)}${p.peer_asn}${p.role ? ` ${esc(p.role)}${p.role_confirmed ? ' ✓' : ''}` : ''} ${esc(p.state)}${up ? (p.direction === 'outbound' ? 'us' : 'peer') : DASH} ${up ? esc(p.local_addr) : DASH}${up ? fmtTime(p.established_at_unix) : DASH} ${up ? age(p.established_at_unix) : DASH}${up ? (p.four_octet_asn ? 'yes' : 'no') : DASH} diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index b42f7ae..ce35b0f 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -18,7 +18,6 @@ cargo-fuzz = true bgp-packet = { path = "../crates/packet" } bgp-primitives = { path = "../crates/primitives" } bgp-fib = { path = "../crates/fib" } -bgp-policy = { path = "../crates/policy" } bgp-rib = { path = "../crates/rib" } bgp-rpki = { path = "../crates/rpki" } bgp-server = { path = "../crates/server" } @@ -36,13 +35,6 @@ test = false doc = false bench = false -[[bin]] -name = "policy" -path = "fuzz_targets/policy.rs" -test = false -doc = false -bench = false - [[bin]] name = "message" path = "fuzz_targets/message.rs" diff --git a/fuzz/README.md b/fuzz/README.md index d323527..76fd887 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -15,7 +15,6 @@ you leave them running. | `rtr` | A stream of RTR PDUs, as an RPKI cache sends them | | `netlink` | Messages from the kernel's routing socket | | `attrs` | The routing table's reading of stored attribute bytes | -| `policy` | The example policy script on routes with any attributes | For the two message targets a finding is either a panic, or a message the decoder accepted that cannot be written and read back as itself. diff --git a/fuzz/fuzz_targets/policy.rs b/fuzz/fuzz_targets/policy.rs deleted file mode 100644 index 3417550..0000000 --- a/fuzz/fuzz_targets/policy.rs +++ /dev/null @@ -1,84 +0,0 @@ -//! The operator's policy script, asked about routes with whatever -//! attributes a peer can send. The example script (testdata/policy.rhai) -//! touches most of what a script can; whatever the route, a function -//! either answers or fails, and never panics. What it changes must still -//! be attributes that can be written. -#![no_main] - -use std::sync::{Arc, LazyLock}; - -use bgp_packet::message::{Message, PathAttribute, UpdateMessage}; -use bgp_policy::{Direction, Hook, Peer, Policy, Subject}; -use bgp_primitives::ip_prefix::IpPrefix; -use bgp_primitives::parser::ParserContext; -use bytes::BytesMut; -use libfuzzer_sys::fuzz_target; - -static HOOKS: LazyLock<(Arc, Vec)> = LazyLock::new(|| { - let policy = Policy::compile(include_str!("../../testdata/policy.rhai")).unwrap(); - let hooks = ["from_peer", "to_peer"] - .into_iter() - .map(|name| policy.hook(name).unwrap()) - .collect(); - (policy, hooks) -}); - -fuzz_target!(|data: &[u8]| { - let Some((selector, attributes)) = data.split_first() else { - return; - }; - let ctx = ParserContext { - four_octet_asn: true, - ibgp: false, - add_path: Vec::new(), - }; - // An UPDATE with no routes withdrawn, these attributes, and one route. - let mut body = vec![0, 0]; - let attributes = &attributes[..attributes.len().min(4000)]; - body.extend_from_slice(&(attributes.len() as u16).to_be_bytes()); - body.extend_from_slice(attributes); - body.extend_from_slice(&[24, 203, 0, 113]); - let Ok(decoded) = UpdateMessage::decode(&ctx, &body) else { - return; - }; - let attributes = decoded.update.path_attributes; - let prefix = IpPrefix::try_from("203.0.113.0/24").unwrap(); - let roles = ["", "customer", "peer", "provider", "route_server"]; - let peer = Peer::new( - "backup-transit", - std::net::Ipv4Addr::new(192, 0, 2, 2).into(), - 64602, - 65000, - roles[usize::from(selector >> 1) % roles.len()], - ); - let (hook, direction) = if selector & 1 == 0 { - (&HOOKS.1[0], Direction::Import) - } else { - (&HOOKS.1[1], Direction::Export) - }; - let subject = Subject { - prefix: &prefix, - attributes: &attributes, - next_hop: None, - direction, - }; - let Ok(verdict) = hook.run(&peer, &subject) else { - return; - }; - if let Some(changed) = verdict.attributes { - let changed: Vec = changed; - let update = UpdateMessage { - withdrawn_routes: vec![], - path_attributes: changed, - announced_routes: vec![], - }; - let mut wire = BytesMut::new(); - // Too long to send is a thing that can be; anything else is not. - if let Err(e) = Message::Update(update).to_wire(&ctx, &mut wire) { - assert!( - matches!(e, bgp_primitives::parser::EncodeError::TooLong { .. }), - "{e}" - ); - } - } -}); diff --git a/fuzz/seeds/policy/in b/fuzz/seeds/policy/in deleted file mode 100644 index 0aa9825..0000000 Binary files a/fuzz/seeds/policy/in and /dev/null differ diff --git a/fuzz/seeds/policy/out b/fuzz/seeds/policy/out deleted file mode 100644 index 1698bfb..0000000 Binary files a/fuzz/seeds/policy/out and /dev/null differ diff --git a/scripts/interop/lab.sh b/scripts/interop/lab.sh index 7e36432..6d94962 100755 --- a/scripts/interop/lab.sh +++ b/scripts/interop/lab.sh @@ -29,7 +29,7 @@ cd "$(dirname "$0")/../.." bgp="${CARGO_TARGET_DIR:-target}/release/bgp" work=/tmp/ferrix-lab pathspace=ferrixlab -all_phases="basics roles rpki refresh flaps restart scale limits hijacks nexthops policy v6only linklocal" +all_phases="basics roles rpki refresh flaps restart scale limits hijacks nexthops v6only linklocal" phases="${PHASES:-$all_phases}" scale_routes="${SCALE_ROUTES:-200000}" @@ -828,77 +828,6 @@ phase_nexthops() { ok "back to normal" } -# The operator's own rules, as a script: routes are marked with where -# they were learned and ranked by it, which changes which of two routes -# is chosen; and what each neighbor is told is decided for each. -phase_policy() { - cat > "$work/policy.rhai" <<'END' -const LEARNED_FROM = #{ peer: "64601:2000", provider: "64601:3000" }; - -// Every route on its way in. Here the provider's routes are the ones to -// prefer, which is the other way about from what selection would do. -fn incoming(route, peer) { - route.remove_community("64601:*"); - route.add_community(LEARNED_FROM[peer.role]); - route.local_pref = if peer.role == "provider" { 300 } else { 100 }; - true -} - -fn to_frr(route, peer) { - route.add_community("64601:777"); - route.prepend(2); - true -} - -// BIRD is not told our IPv4 prefix. -fn to_bird(route, peer) { !route.within("10.1.0.0/24") } -END - dut_down TERM - dut_config PEER 2000000 "" \ - 'export { function: "to_frr" }' 'export { function: "to_bird" }' \ - 'policy { path: "policy.rhai" import_function: "incoming" }' - "$bgp" --check "$work/dut.textpb" | grep -q "frr4: import incoming, export to_frr" \ - || fail "--check does not say which functions are whose: $("$bgp" --check "$work/dut.textpb" 2>&1)" - dut_up - # A prefix nobody has said anything about in the RPKI, from both; of - # the two, FRR's would be chosen. - echo "route 10.77.0.0/24 blackhole;" > "$work/bird-both.conf" - bird_config "$work/bird-both.conf" - bird_ctl configure >/dev/null - frr_configure "router bgp 64602" "address-family ipv4 unicast" "network 10.77.0.0/24" - wait_for "four sessions" established 4 - wait_for "FRR's route for it" dut_has 10.77.0.0/24 frr4 - wait_for "BIRD's route for it" dut_has 10.77.0.0/24 bird4 - - # In: marked by where they were learned, and ranked. - local marks - marks=$(api "/api/routes/received?prefix=10.77.0.0/24" | jq -c '[.routes[] | {peer: .peer_name, marks: (tostring | [scan("64601:[0-9]+")] | unique)}] | sort_by(.peer)') - [ "$marks" = '[{"peer":"bird4","marks":["64601:3000"]},{"peer":"frr4","marks":["64601:2000"]}]' ] \ - || fail "the routes are marked $marks" - wait_for "BIRD's route to be the one chosen" best_is 10.77.0.0/24 bird4 - wait_for "it in the kernel" dut_kernel -4 "10.77.0.0/24 via 10.9.0.3 dev ix0 metric 20" - peer_is frr4 '.import_function == "incoming" and .export_function == "to_frr"' >/dev/null \ - || fail "the status does not say which functions are the peer's" - ok "routes are marked with where they were learned, and the provider's is chosen and installed" - - # Out: FRR is told our route with a longer path and a mark; BIRD is not - # told it at all, and is told the IPv6 one. - wait_for "our route at FRR" bash -c "sudo ip netns exec lab-frr vtysh -N $pathspace -c 'show bgp ipv4 unicast 10.1.0.0/24 json' 2>/dev/null | jq -e '.paths[0] | .aspath.string == \"64601 64601 64601\" and (.community.string | contains(\"64601:777\")) and (.community.string | contains(\"64601:100\"))'" - wait_for "our IPv6 route at BIRD" bird_has 2001:db8:1::/48 - sleep 1 - bird_has 10.1.0.0/24 && fail "BIRD was told the route its function refuses" - ok "FRR is told our route with a longer path and a mark; BIRD is not told it" - - frr_configure "router bgp 64602" "address-family ipv4 unicast" "no network 10.77.0.0/24" - bird_config - bird_ctl configure >/dev/null - dut_down TERM - dut_config - dut_up - converged - ok "back to normal" -} - # The exchange LAN loses its IPv4 addresses. One session over IPv6 with # each router carries both families, and IPv4 routes have IPv6 next hops # (RFC 8950), in BGP and in the kernel. Run after the rest: the lab stays diff --git a/testdata/policy.rhai b/testdata/policy.rhai deleted file mode 100644 index 054b397..0000000 --- a/testdata/policy.rhai +++ /dev/null @@ -1,61 +0,0 @@ -// An example policy script; see "Policy scripts" in bin/bgp/README.md for -// everything a function can read and change. -// -// A function is given a route and the peer it comes from (or goes to). It -// may change the route, and returns whether the route is taken (or -// announced). Which function is for which peer is said in the -// configuration: import { function: "from_peer" }. -// -// Constants defined here are for every function below. - -// Where a route was learned: what the rules for announcing it go by. -const LEARNED_FROM = #{ - customer: "65000:1000", - peer: "65000:2000", - provider: "65000:3000", - route_server: "65000:2000", -}; - -// A customer's route is preferred to a peer's, and that to a provider's: -// the first earns money, the last costs it. -const PREFERENCE = #{ customer: 300, peer: 200, route_server: 200, provider: 100 }; - -const BLACKHOLE = "65535:666"; -const NO_EXPORT = "65535:65281"; - -// Routes on their way in. -fn from_peer(route, peer) { - // Not ours to act on, from just anyone. - if route.has_community(BLACKHOLE) { return false; } - - // Marks in our own range mean something here: nobody else's word for - // them is taken. - route.remove_community("65000:*"); - if peer.role in LEARNED_FROM { - route.add_community(LEARNED_FROM[peer.role]); - route.local_pref = PREFERENCE[peer.role]; - } - - // A peer can ask to be a last resort: 65000:80 on a route of its own. - if route.has_community(peer.asn % 65536, 80) { - route.local_pref = 80; - } - true -} - -// Routes on their way out. -fn to_peer(route, peer) { - if route.has_community(NO_EXPORT) { return false; } - - // Only customers are told what we learned from peers and providers. - let from_above = route.has_community("65000:2000") || route.has_community("65000:3000"); - if from_above && peer.role != "customer" { return false; } - - // Our marks are our own business. - route.remove_community("65000:*"); - - // Traffic from this one is wanted by another way: the path looks - // longer to it. - if peer.name == "backup-transit" { route.prepend(2); } - true -} diff --git a/testdata/server.textpb b/testdata/server.textpb index 511c166..46b311a 100644 --- a/testdata/server.textpb +++ b/testdata/server.textpb @@ -48,13 +48,7 @@ peers { # as generated from the routing registries. # prefixes { prefix: "193.0.0.0/21" max_length: 24 } # origin_asns: 3333 - # And then the operator's own rules: a function of the policy - # script (see `policy` below), which may refuse the route or mark - # and rank it. - # function: "from_peer" } - # What the peer is told also goes through a function of the script. - # export { function: "to_peer" } } connect_retry_seconds: 5 @@ -71,11 +65,6 @@ originate { large_communities: "65000:1:2" } -# The operator's own rules for routes, as a script beside this file; see -# policy.rhai here for an example, and "Policy scripts" in -# bin/bgp/README.md. `bgp --check` reads it and says what is wrong. -# policy { path: "policy.rhai" } - # Validation with the RPKI: refuse routes whose origin AS is not the one the # holder of the addresses authorised, and, from peers with a role, routes # whose AS path is a leak. Point it at a validator's RTR server.