diff --git a/bin/bgp/README.md b/bin/bgp/README.md index 29c4bad..c2a454b 100644 --- a/bin/bgp/README.md +++ b/bin/bgp/README.md @@ -94,6 +94,30 @@ the reasons `RPKI invalid` and `ASPA invalid`. The connection to the cache is plain TCP, without authentication: run the validator on the same host or on a network you trust. +## Roles and route leaks (RFC 9234) + +`role` on a peer says what the peer is to us: `PROVIDER`, `CUSTOMER`, `PEER`, +`ROUTE_SERVER` or `ROUTE_SERVER_CLIENT`. It is not for peers in our own AS. +With it: + +- **The two ends have to agree.** Our OPEN says what we are to the peer (its + provider, if it is our customer). A peer whose OPEN says it is something + else than configured is refused with OPEN Message Error / Role Mismatch: + one side thinking "customer" and the other "peer" is how leaks happen. A + peer that says nothing (it does not implement the RFC) is accepted, unless + `require_role: true`. The status page shows the role, with a tick when the + peer confirmed it. +- **Routes are marked "only to customer"** (the OTC attribute) when they come + in from a provider, a peer or a route server, and when ours go out to a + customer, a peer or a route server client. The mark stays on the route. +- **A marked route coming the wrong way is a leak and is refused**: from a + customer or a route server client always, and from a peer unless the peer + put the mark there itself. The reason shown is `route leak (Only to + Customer)`. No setting turns this off, as the RFC requires. + +This catches leaks even where nobody on the path has an ASPA, as long as the +networks on either side of the leaker mark their routes. + ## AS path verification (ASPA) Origin validation says who may originate a prefix. It says nothing about the diff --git a/crates/integration-tests/tests/add_path.rs b/crates/integration-tests/tests/add_path.rs index 175ad56..8188c6d 100644 --- a/crates/integration-tests/tests/add_path.rs +++ b/crates/integration-tests/tests/add_path.rs @@ -49,6 +49,7 @@ async fn server_offering( add_path, import: ImportPolicy::permissive(), role: None, + require_role: false, }]; 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 8a2d85f..8a2226d 100644 --- a/crates/integration-tests/tests/admission.rs +++ b/crates/integration-tests/tests/admission.rs @@ -27,6 +27,7 @@ fn peer_at(endpoint: SocketAddr) -> PeerConfig { add_path: None, import: ImportPolicy::permissive(), role: None, + require_role: false, } } diff --git a/crates/integration-tests/tests/aspa.rs b/crates/integration-tests/tests/aspa.rs index 66569ea..8325b94 100644 --- a/crates/integration-tests/tests/aspa.rs +++ b/crates/integration-tests/tests/aspa.rs @@ -65,6 +65,7 @@ async fn server_with( add_path: None, import, role, + require_role: false, }]; }) .await?; diff --git a/crates/integration-tests/tests/collision.rs b/crates/integration-tests/tests/collision.rs index 4624fb4..2c7c8c6 100644 --- a/crates/integration-tests/tests/collision.rs +++ b/crates/integration-tests/tests/collision.rs @@ -45,6 +45,7 @@ async fn server_dialing(listener: &TcpListener) -> Result { add_path: None, import: ImportPolicy::permissive(), role: None, + require_role: false, }]; }) .await diff --git a/crates/integration-tests/tests/debugging.rs b/crates/integration-tests/tests/debugging.rs index fe3b42c..b6dafba 100644 --- a/crates/integration-tests/tests/debugging.rs +++ b/crates/integration-tests/tests/debugging.rs @@ -176,6 +176,7 @@ async fn test_refused_routes_can_be_seen_and_counted() -> Result<()> { ..ImportPolicy::default() }, role: None, + require_role: false, }]; }) .await?; diff --git a/crates/integration-tests/tests/dialer.rs b/crates/integration-tests/tests/dialer.rs index e595220..313ce7a 100644 --- a/crates/integration-tests/tests/dialer.rs +++ b/crates/integration-tests/tests/dialer.rs @@ -36,6 +36,7 @@ async fn server_dialing(listener: &TcpListener, remote_asn: u32) -> Result Result<()> { add_path: None, import: ImportPolicy::permissive(), role: None, + require_role: false, }]; }) .await?; @@ -242,6 +244,7 @@ async fn test_local_address_is_used_as_the_source() -> Result<()> { add_path: None, import: ImportPolicy::permissive(), role: None, + require_role: false, }]; }) .await?; @@ -261,6 +264,7 @@ async fn test_local_address_is_used_as_the_source() -> Result<()> { add_path: None, import: ImportPolicy::permissive(), role: None, + require_role: false, }]; }) .await?; diff --git a/crates/integration-tests/tests/import_policy.rs b/crates/integration-tests/tests/import_policy.rs index bab0938..e721e4d 100644 --- a/crates/integration-tests/tests/import_policy.rs +++ b/crates/integration-tests/tests/import_policy.rs @@ -49,6 +49,7 @@ async fn server_with(import: ImportPolicy) -> Result<(TestServer, TcpListener)> add_path: None, import, role: None, + require_role: false, }]; }) .await?; diff --git a/crates/integration-tests/tests/interop.rs b/crates/integration-tests/tests/interop.rs index d7d410c..8a23679 100644 --- a/crates/integration-tests/tests/interop.rs +++ b/crates/integration-tests/tests/interop.rs @@ -126,6 +126,7 @@ async fn test_ipv6_prefix_update_shape() -> Result<()> { next_hop: nh.into(), four_octet_asn: true, ibgp: false, + role: None, }, ) .expect("ipv6 over ipv6"); diff --git a/crates/integration-tests/tests/negotiation.rs b/crates/integration-tests/tests/negotiation.rs index 015e0e5..8730cba 100644 --- a/crates/integration-tests/tests/negotiation.rs +++ b/crates/integration-tests/tests/negotiation.rs @@ -192,6 +192,7 @@ async fn test_no_family_in_common_refuses_the_session() -> Result<()> { add_path: None, import: ImportPolicy::permissive(), role: None, + require_role: false, }]; // Keep the dialer out of the way. c.connect_retry = Duration::from_secs(3600); diff --git a/crates/integration-tests/tests/roles.rs b/crates/integration-tests/tests/roles.rs new file mode 100644 index 0000000..16e79ae --- /dev/null +++ b/crates/integration-tests/tests/roles.rs @@ -0,0 +1,407 @@ +//! Roles and route leak prevention (RFC 9234), end to end: what the two +//! ends say they are to each other, and what becomes of routes that are +//! marked as only to go to customers. + +use std::net::{Ipv4Addr, Ipv6Addr}; +use std::time::Duration; + +use bgp_integration_tests::test_peer::TestPeer; +use bgp_integration_tests::test_server::TestServer; +use bgp_packet::errors::{BgpError, OpenMsgSubcode}; +use bgp_packet::message::{ + AsPathAttribute, Message, MpReachNlriPathAttribute, NextHopPathAttribute, NlriNextHop, + OriginPathAttribute, PathAttribute, UpdateMessage, +}; +use bgp_packet::open::{BgpRole, Capability}; +use bgp_primitives::constants::AfiSafi; +use bgp_primitives::ip_prefix::IpPrefix; +use bgp_rib::LocalRoute; +use bgp_server::config::PeerConfig; +use bgp_server::family::FamilySet; +use bgp_server::policy::{ImportPolicy, PeerRole}; +use eyre::{Result, bail}; +use serde_json::Value; +use tokio::net::TcpListener; + +const LOCAL_ASN: u32 = 1299; +const PEER_ASN: u32 = 3333; +const TIMEOUT: Duration = Duration::from_secs(5); + +fn prefix(s: &str) -> IpPrefix { + IpPrefix::try_from(s).unwrap() +} + +/// What a test holds on to: the server, and the listener its dialling +/// goes to. +struct Setup { + server: TestServer, + _listener: TcpListener, +} + +/// A server with one configured peer at the address the test connects +/// from, which is `role` to it, and which originates 193.0.0.0/21. +async fn server_with(role: Option, require_role: bool) -> Result { + let listener = TcpListener::bind((Ipv4Addr::LOCALHOST, 0)).await?; + let endpoint = listener.local_addr()?; + let server = TestServer::start_with(LOCAL_ASN, |c| { + c.accept_unconfigured_peers = false; + c.originate = vec![LocalRoute::new(prefix("193.0.0.0/21"), Default::default())]; + c.peers = vec![PeerConfig { + name: "r1".into(), + endpoint, + remote_asn: PEER_ASN, + hold_time_secs: 90, + local_address: None, + families: FamilySet::supported(), + add_path: None, + import: ImportPolicy::default(), + role, + require_role, + }]; + }) + .await?; + Ok(Setup { + server, + _listener: listener, + }) +} + +/// The capabilities of an ordinary peer, and with `role` what it says it +/// is. +fn capabilities(role: Option) -> Vec { + let mut capabilities = vec![ + Capability::FourByteAsn { asn: PEER_ASN }, + Capability::multiprotocol(AfiSafi::IPV4_UNICAST), + Capability::multiprotocol(AfiSafi::IPV6_UNICAST), + Capability::RouteRefresh, + ]; + capabilities.extend(role.map(|role| Capability::BgpRole { role })); + capabilities +} + +/// A session with the server, on which the peer says it is `says`. +async fn established(server: &TestServer, says: Option) -> Result { + let mut peer = TestPeer::connect(server.addr).await?; + peer.handshake_with(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90, capabilities(says)) + .await?; + tokio::time::timeout(TIMEOUT, async { + while server.registry.list().is_empty() { + tokio::time::sleep(Duration::from_millis(5)).await; + } + }) + .await?; + Ok(peer) +} + +/// Announces `prefixes` with the AS path `path` and, with `marked_by`, an +/// Only to Customer attribute with that AS number. +async fn announce( + peer: &mut TestPeer, + prefixes: &[&str], + path: &[u32], + marked_by: Option, +) -> Result<()> { + let mut path_attributes = vec![ + PathAttribute::Origin(OriginPathAttribute::IGP), + AsPathAttribute::from_asns(path.to_vec()), + PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), + ]; + path_attributes.extend(marked_by.map(PathAttribute::OnlyToCustomer)); + peer.send(&Message::Update(UpdateMessage { + withdrawn_routes: vec![], + path_attributes, + announced_routes: prefixes.iter().map(|p| prefix(p).into()).collect(), + })) + .await +} + +/// Waits until the table holds `count` routes, and returns the mark each +/// has, by prefix. +async fn marks(server: &TestServer, count: usize) -> Result)>> { + tokio::time::timeout(TIMEOUT, async { + while server.received_routes().len() != count { + tokio::time::sleep(Duration::from_millis(5)).await; + } + }) + .await + .map_err(|_| { + eyre::eyre!( + "expected {count} routes, the table has {:?}", + server.received_routes() + ) + })?; + let mut marks: Vec<(String, Option)> = server + .received_routes() + .iter() + .map(|route| { + let mark = route + .attributes + .iter() + .find_map(|attribute| match attribute { + PathAttribute::OnlyToCustomer(asn) => Some(*asn), + _ => None, + }); + (route.prefix.to_string(), mark) + }) + .collect(); + marks.sort(); + Ok(marks) +} + +/// The role the server's OPEN announces. +fn announced_role(open: &bgp_packet::open::OpenMessage) -> Option { + open.capabilities().find_map(|capability| match capability { + Capability::BgpRole { role } => Some(*role), + _ => None, + }) +} + +/// Both ends say what they are, and it fits: the session comes up, and is +/// shown as one whose peer confirmed its role. +#[tokio::test] +async fn test_roles_that_fit_together_are_confirmed() -> Result<()> { + let Setup { server, .. } = server_with(Some(PeerRole::Customer), false).await?; + let (base, _http) = server.serve_http().await?; + let mut peer = TestPeer::connect(server.addr).await?; + let open = peer + .handshake_with( + PEER_ASN, + Ipv4Addr::new(10, 0, 0, 2), + 90, + capabilities(Some(BgpRole::Customer)), + ) + .await?; + // The peer is our customer: we are its provider. + assert_eq!(announced_role(&open), Some(BgpRole::Provider)); + + tokio::time::timeout(TIMEOUT, async { + while server.registry.list().is_empty() { + tokio::time::sleep(Duration::from_millis(5)).await; + } + }) + .await?; + let peers: Value = reqwest::get(format!("{base}/api/peers")) + .await? + .json() + .await?; + assert_eq!(peers[0]["role"], "customer"); + assert_eq!(peers[0]["role_confirmed"], true); + Ok(()) +} + +/// The commonest way to a leak is the two ends not agreeing on what they +/// are to each other. A peer that says it is something else than it is +/// configured as is refused, with the error RFC 9234 gives for it. +#[tokio::test] +async fn test_a_peer_that_says_it_is_something_else_is_refused() -> Result<()> { + let Setup { server, .. } = server_with(Some(PeerRole::Customer), false).await?; + let mut peer = TestPeer::connect(server.addr).await?; + // It thinks it is our peer; we have it down as a customer. + peer.send_open_with( + PEER_ASN, + Ipv4Addr::new(10, 0, 0, 2), + 90, + capabilities(Some(BgpRole::Peer)), + ) + .await?; + peer.recv_open().await?; + match tokio::time::timeout(TIMEOUT, peer.recv()).await?? { + Message::Notification(n) => { + assert_eq!(n.error, BgpError::OpenMsg(OpenMsgSubcode::RoleMismatch)); + // Subcode 11 on the wire. + assert_eq!(n.error.code_subcode(), (2, 11)); + } + other => bail!("expected NOTIFICATION, got {other:?}"), + } + assert!(server.registry.list().is_empty()); + Ok(()) +} + +/// A peer that says nothing does not implement RFC 9234. It is taken at +/// our word for what it is, unless the configuration requires it to say. +#[tokio::test] +async fn test_a_peer_that_does_not_say_is_refused_only_where_required() -> Result<()> { + let Setup { server, .. } = server_with(Some(PeerRole::Customer), false).await?; + let (base, _http) = server.serve_http().await?; + let _peer = established(&server, None).await?; + let peers: Value = reqwest::get(format!("{base}/api/peers")) + .await? + .json() + .await?; + assert_eq!(peers[0]["role"], "customer"); + assert_eq!(peers[0]["role_confirmed"], false); + + let Setup { server, .. } = server_with(Some(PeerRole::Customer), true).await?; + let mut peer = TestPeer::connect(server.addr).await?; + peer.send_open_with(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90, capabilities(None)) + .await?; + peer.recv_open().await?; + match tokio::time::timeout(TIMEOUT, peer.recv()).await?? { + Message::Notification(n) => { + assert_eq!(n.error, BgpError::OpenMsg(OpenMsgSubcode::RoleMismatch)); + } + other => bail!("expected NOTIFICATION, got {other:?}"), + } + Ok(()) +} + +/// A route marked as only to go to customers has been handed down or +/// across once. Coming up from a customer it is a leak, and is refused +/// whatever else the peer's policy takes. +#[tokio::test] +async fn test_a_marked_route_from_a_customer_is_a_leak() -> Result<()> { + let Setup { server, .. } = server_with(Some(PeerRole::Customer), false).await?; + let mut peer = established(&server, Some(BgpRole::Customer)).await?; + + // The customer's own route, and one its other provider gave it. + announce(&mut peer, &["194.0.0.0/24"], &[PEER_ASN], None).await?; + announce( + &mut peer, + &["8.8.8.0/24"], + &[PEER_ASN, 3356, 15169], + Some(3356), + ) + .await?; + // A customer's routes may go anywhere: they are not marked on the way + // in. + assert_eq!( + marks(&server, 1).await?, + [("194.0.0.0/24".to_string(), None)] + ); + tokio::time::timeout(TIMEOUT, async { + while server.refusals.peers().is_empty() { + tokio::time::sleep(Duration::from_millis(5)).await; + } + }) + .await?; + let refused = &server.refusals.peers()[0].1; + assert_eq!(refused.total(), 1); + assert_eq!(refused.recent()[0].prefix, prefix("8.8.8.0/24")); + assert_eq!( + refused.recent()[0].reason.as_str(), + "route leak (Only to Customer)" + ); + Ok(()) +} + +/// From a peer a marked route is in order only if the peer marked it +/// itself, on its way to us. A mark of someone else's means the peer got +/// the route from above or from another peer. +#[tokio::test] +async fn test_a_route_marked_by_someone_else_is_a_leak_from_a_peer() -> Result<()> { + let Setup { server, .. } = server_with(Some(PeerRole::Peer), false).await?; + let mut peer = established(&server, Some(BgpRole::Peer)).await?; + + announce(&mut peer, &["194.0.0.0/24"], &[PEER_ASN], Some(PEER_ASN)).await?; + announce( + &mut peer, + &["8.8.8.0/24"], + &[PEER_ASN, 3356, 15169], + Some(3356), + ) + .await?; + // A peer that does not mark its routes has them marked for it. + announce(&mut peer, &["195.0.0.0/24"], &[PEER_ASN, 2121], None).await?; + assert_eq!( + marks(&server, 2).await?, + [ + ("194.0.0.0/24".to_string(), Some(PEER_ASN)), + ("195.0.0.0/24".to_string(), Some(PEER_ASN)), + ] + ); + assert_eq!(server.refusals.peers()[0].1.total(), 1); + Ok(()) +} + +/// A route from a provider is marked on the way in, if it is not already: +/// from here it may only go down. That holds for both address families, +/// and whether or not the peer's attributes could be stored as they came. +#[tokio::test] +async fn test_routes_from_a_provider_are_marked_on_the_way_in() -> Result<()> { + for four_octet in [true, false] { + let Setup { server, .. } = server_with(Some(PeerRole::Provider), false).await?; + let mut peer = TestPeer::connect(server.addr).await?; + let mut offered = capabilities(Some(BgpRole::Provider)); + if !four_octet { + offered.retain(|capability| !matches!(capability, Capability::FourByteAsn { .. })); + } + peer.handshake_with(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90, offered) + .await?; + + announce(&mut peer, &["194.0.0.0/24"], &[PEER_ASN, 2121], None).await?; + // One that someone further up has marked already keeps that mark. + announce( + &mut peer, + &["8.8.8.0/24"], + &[PEER_ASN, 3356, 15169], + Some(3356), + ) + .await?; + peer.send(&Message::Update(UpdateMessage { + withdrawn_routes: vec![], + path_attributes: vec![ + PathAttribute::Origin(OriginPathAttribute::IGP), + AsPathAttribute::from_asns(vec![PEER_ASN, 2121]), + PathAttribute::MpReachNlri(MpReachNlriPathAttribute { + family: AfiSafi::IPV6_UNICAST, + next_hop: NlriNextHop::Ipv6(Ipv6Addr::new(0x2001, 0x7f8, 0, 0, 0, 0, 0, 1)), + prefixes: vec![prefix("2a00:1450::/32").into()], + }), + ], + announced_routes: vec![], + })) + .await?; + assert_eq!( + marks(&server, 3).await?, + [ + ("194.0.0.0/24".to_string(), Some(PEER_ASN)), + ("2a00:1450::/32".to_string(), Some(PEER_ASN)), + ("8.8.8.0/24".to_string(), Some(3356)), + ], + "four-octet AS numbers: {four_octet}" + ); + assert!(server.refusals.peers().is_empty()); + } + Ok(()) +} + +/// Our own routes are marked on their way down to a customer or across to +/// a peer, and not on their way up to a provider. +#[tokio::test] +async fn test_our_routes_are_marked_on_the_way_down_or_across() -> Result<()> { + for (peer_is, says, marked) in [ + (PeerRole::Customer, BgpRole::Customer, true), + (PeerRole::Peer, BgpRole::Peer, true), + ( + PeerRole::RouteServerClient, + BgpRole::RouteServerClient, + true, + ), + (PeerRole::Provider, BgpRole::Provider, false), + (PeerRole::RouteServer, BgpRole::RouteServer, false), + ] { + let Setup { server, .. } = server_with(Some(peer_is), false).await?; + let mut peer = established(&server, Some(says)).await?; + let update = tokio::time::timeout(TIMEOUT, peer.recv_update()).await??; + assert_eq!(update.announced_routes, [prefix("193.0.0.0/21")]); + let mark = update + .path_attributes + .iter() + .find_map(|attribute| match attribute { + PathAttribute::OnlyToCustomer(asn) => Some(*asn), + _ => None, + }); + assert_eq!(mark, marked.then_some(LOCAL_ASN), "to our {peer_is:?}"); + } + // Nothing is said to a peer whose role is not known. + let Setup { server, .. } = server_with(None, false).await?; + let mut peer = established(&server, None).await?; + let update = tokio::time::timeout(TIMEOUT, peer.recv_update()).await??; + assert!( + !update + .path_attributes + .iter() + .any(|attribute| matches!(attribute, PathAttribute::OnlyToCustomer(_))) + ); + Ok(()) +} diff --git a/crates/integration-tests/tests/rpki.rs b/crates/integration-tests/tests/rpki.rs index 7f6030d..58b6528 100644 --- a/crates/integration-tests/tests/rpki.rs +++ b/crates/integration-tests/tests/rpki.rs @@ -74,6 +74,7 @@ async fn server_with( add_path: None, import, role: None, + require_role: false, }]; }) .await?; diff --git a/crates/packet/src/errors.rs b/crates/packet/src/errors.rs index c2a5354..50cc7e5 100644 --- a/crates/packet/src/errors.rs +++ b/crates/packet/src/errors.rs @@ -128,8 +128,9 @@ pub enum OpenMsgSubcode { #[error("Unsupported capability")] UnsupportedCapability = 7, + /// RFC 9234 §4.2. #[error("Role mismatch")] - RoleMismatch = 8, + RoleMismatch = 11, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Error, Serialize, FromRepr)] diff --git a/crates/packet/src/message.rs b/crates/packet/src/message.rs index d19e364..937593c 100644 --- a/crates/packet/src/message.rs +++ b/crates/packet/src/message.rs @@ -405,6 +405,7 @@ pub mod attr_type { pub const AS4_PATH: u8 = 17; pub const AS4_AGGREGATOR: u8 = 18; pub const LARGE_COMMUNITIES: u8 = 32; + pub const ONLY_TO_CUSTOMER: u8 = 35; } #[derive(Debug, Clone, Serialize, PartialEq, Eq)] @@ -424,6 +425,10 @@ pub enum PathAttribute { /// speakers that only handle two-octet ones (RFC 6793 §3). As4Path(AsPathAttribute), LargeCommunities(LargeCommunitiesPathAttribute), + /// Only to Customer (RFC 9234 §5): the route has been handed to a + /// customer, a peer or a route server client by the AS with this + /// number, and from there on may only go down to customers. + OnlyToCustomer(u32), /// An attribute this implementation does not interpret, kept as it /// came so that it can be passed on. UnknownPathAttribute { @@ -447,7 +452,7 @@ pub(crate) fn defined_flags(type_code: u8) -> Option<(bool, bool)> { OPTIONAL_NON_TRANSITIVE } AGGREGATOR | COMMUNITIES | EXTENDED_COMMUNITIES | AS4_PATH | AS4_AGGREGATOR - | LARGE_COMMUNITIES => OPTIONAL_TRANSITIVE, + | LARGE_COMMUNITIES | ONLY_TO_CUSTOMER => OPTIONAL_TRANSITIVE, _ => return None, }) } @@ -547,6 +552,7 @@ impl PathAttribute { Self::ExtendedCommunities(_) => EXTENDED_COMMUNITIES, Self::As4Path(_) => AS4_PATH, Self::LargeCommunities(_) => LARGE_COMMUNITIES, + Self::OnlyToCustomer(_) => ONLY_TO_CUSTOMER, Self::UnknownPathAttribute { type_code, .. } => *type_code, } } @@ -615,6 +621,15 @@ impl PathAttribute { LARGE_COMMUNITIES => { Self::LargeCommunities(whole(LargeCommunitiesPathAttribute::from_wire(value)?)?) } + ONLY_TO_CUSTOMER => { + let asn = <[u8; 4]>::try_from(value).map_err(|_| { + malformed(format!( + "Only to Customer is {} octets long, not four", + value.len() + )) + })?; + Self::OnlyToCustomer(u32::from_be_bytes(asn)) + } _ => Self::UnknownPathAttribute { flags, type_code, @@ -671,12 +686,29 @@ impl PathAttribute { Self::ExtendedCommunities(a) => a.to_wire(out), Self::As4Path(a) => a.to_wire_with_width(true, out)?, Self::LargeCommunities(a) => a.to_wire(out), + Self::OnlyToCustomer(asn) => out.put_u32(*asn), Self::UnknownPathAttribute { payload, .. } => out.put_slice(payload), } Ok(()) } } +/// An Only to Customer attribute with this AS number, as it is on the +/// wire: for adding to attributes that are kept as bytes. +pub fn only_to_customer_wire(asn: u32) -> [u8; 7] { + const OPTIONAL_TRANSITIVE: u8 = 0xc0; + let [a, b, c, d] = asn.to_be_bytes(); + [ + OPTIONAL_TRANSITIVE, + attr_type::ONLY_TO_CUSTOMER, + 4, + a, + b, + c, + d, + ] +} + type Parsed<'a, T> = IResult<&'a [u8], T, BgpParserError<&'a [u8]>>; fn malformed<'a>(detail: String) -> nom::Err> { diff --git a/crates/packet/src/update_decode.rs b/crates/packet/src/update_decode.rs index 259bc2b..2282702 100644 --- a/crates/packet/src/update_decode.rs +++ b/crates/packet/src/update_decode.rs @@ -478,6 +478,7 @@ mod tests { const COMMUNITIES: u8 = 8; const EXTENDED_COMMUNITIES: u8 = 16; const LARGE_COMMUNITY: u8 = 32; + const ONLY_TO_CUSTOMER: u8 = 35; const WELL_KNOWN: u8 = 0x40; const OPTIONAL: u8 = 0x80; @@ -626,6 +627,31 @@ mod tests { assert_eq!(decoded.attributes_wire, None); } + #[test] + fn only_to_customer_is_read_and_kept_as_it_came() { + let wire = crate::message::only_to_customer_wire(3333); + assert_eq!( + wire.to_vec(), + attr(OPTIONAL_TRANSITIVE, ONLY_TO_CUSTOMER, &[0, 0, 0x0d, 0x05]) + ); + let decoded = announce_with(&ebgp(), wire.to_vec()); + assert!(decoded.errors.is_empty()); + assert_eq!( + decoded.update.path_attributes.last(), + Some(&PathAttribute::OnlyToCustomer(3333)) + ); + assert_eq!( + decoded.attributes_wire, + Some([mandatory().concat(), wire.to_vec()].concat()) + ); + // And is written as it is read. + let mut out = bytes::BytesMut::new(); + PathAttribute::OnlyToCustomer(3333) + .to_wire(&ebgp(), &mut out) + .unwrap(); + assert_eq!(out[..], wire); + } + #[test] fn an_update_that_announces_nothing_needs_no_attributes() { // End-of-RIB, and a plain withdrawal. @@ -664,6 +690,17 @@ mod tests { MED, attr(OPTIONAL_TRANSITIVE, MED, &[0, 0, 0, 50]), ), + // RFC 9234 §5. + ( + "Only to Customer of two octets", + ONLY_TO_CUSTOMER, + attr(OPTIONAL_TRANSITIVE, ONLY_TO_CUSTOMER, &[0xfd, 0xe8]), + ), + ( + "Only to Customer of eight octets", + ONLY_TO_CUSTOMER, + attr(OPTIONAL_TRANSITIVE, ONLY_TO_CUSTOMER, &[0; 8]), + ), ]; for (case, type_code, extra) in cases { let decoded = announce_with(&ebgp(), extra); diff --git a/crates/packet/tests/properties.rs b/crates/packet/tests/properties.rs index 3117dd4..ba0b9ba 100644 --- a/crates/packet/tests/properties.rs +++ b/crates/packet/tests/properties.rs @@ -189,7 +189,7 @@ fn mp_unreach(session: Session) -> impl Strategy impl Strategy .collect(), }) })), + proptest::option::of(any::().prop_map(PathAttribute::OnlyToCustomer)), unknown_attributes(), ); (mandatory, optional).prop_map(|(mut attributes, optional)| { - let (med, atomic, aggregator, communities, reach, unreach, extended, as4, large, unknown) = - optional; + let ( + med, + atomic, + aggregator, + communities, + reach, + unreach, + extended, + as4, + large, + otc, + unknown, + ) = optional; attributes.extend( [ med, @@ -307,6 +319,7 @@ fn path_attributes(session: Session) -> impl Strategy extended, as4, large, + otc, ] .into_iter() .flatten(), diff --git a/crates/rib/src/table/attrs.rs b/crates/rib/src/table/attrs.rs index ea27962..5a18b31 100644 --- a/crates/rib/src/table/attrs.rs +++ b/crates/rib/src/table/attrs.rs @@ -358,13 +358,28 @@ impl AttrStore { wire: &[u8], next_hop: Option<&NlriNextHop>, ) -> Option> { - let Some(next_hop) = next_hop else { + self.intern_received_with(wire, &[], next_hop) + } + + /// As [`AttrStore::intern_received`], with attributes of our own after + /// the peer's: `added` is one or more whole attributes in the same + /// form, of types that `wire` does not have. + pub fn intern_received_with( + &self, + wire: &[u8], + added: &[u8], + next_hop: Option<&NlriNextHop>, + ) -> Option> { + if added.is_empty() && next_hop.is_none() { return self.intern_wire(wire); - }; - let mut with_next_hop = BytesMut::with_capacity(wire.len() + 36); - with_next_hop.put_slice(wire); - encode_next_hop(next_hop, &mut with_next_hop); - self.intern_wire(&with_next_hop) + } + let mut whole = BytesMut::with_capacity(wire.len() + added.len() + 36); + whole.put_slice(wire); + whole.put_slice(added); + if let Some(next_hop) = next_hop { + encode_next_hop(next_hop, &mut whole); + } + self.intern_wire(&whole) } /// How many distinct sets are stored. diff --git a/crates/server/proto/config.proto b/crates/server/proto/config.proto index 6783873..6d11dba 100644 --- a/crates/server/proto/config.proto +++ b/crates/server/proto/config.proto @@ -146,12 +146,29 @@ message Peer { // apply and there is no limit and no list. ImportPolicy import = 8; - // What the peer is to us. It decides how the AS paths of its routes are - // verified against the ASPAs, where rpki is configured: a route from a - // customer, a peer or a route server must have come up through - // providers all the way; one from a provider may have come up and then - // down. Left out, the paths of the peer's routes are not verified. + // What the peer is to us. Not for a peer in our own AS. With it set: + // + // - Our OPEN says what we are to the peer, and a peer that says it is + // something else than configured here is refused (RFC 9234). One side + // having the other down as a customer and the other as a peer is how + // leaks come about. + // - Routes are marked "only to customer" as they come in from a provider, + // a peer or a route server, and as they go out to a customer, a peer + // or a route server client. A route with that mark that comes back up + // from a customer, or across from a second peer, is a leak and is + // refused. This cannot be turned off. + // - Where rpki is configured, the AS paths of the peer's routes are + // verified against the ASPAs: a route from a customer, a peer or a + // route server must have come up through providers all the way; one + // from a provider may have come up and then down. + // + // Left out, none of this is done for the peer. Role role = 9; + + // Refuse the peer unless its OPEN says what it is. By default a 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 a peer is to us. diff --git a/crates/server/src/advertise.rs b/crates/server/src/advertise.rs index 45a0c7e..b72ea98 100644 --- a/crates/server/src/advertise.rs +++ b/crates/server/src/advertise.rs @@ -34,6 +34,7 @@ use tokio::sync::broadcast::error::RecvError; use tokio::task::JoinHandle; use tracing::{debug, info, warn}; +use crate::policy::PeerRole; use crate::session::{PeerId, SessionClosed, SessionEvent, SessionHandle, SessionRegistry}; /// Starts the advertiser. It hears of every session that comes up after @@ -231,6 +232,7 @@ impl Exporter { next_hop: session.local_addr.ip(), four_octet_asn: session.four_octet_asn, ibgp: session.ibgp, + role: session.role, }; let Some(update) = build_announce(route, &towards) else { debug!( @@ -272,6 +274,8 @@ pub struct Towards { pub four_octet_asn: bool, /// Whether the peer is in our own AS. pub ibgp: bool, + /// What the peer is to us, if that is known. + pub role: Option, } /// Builds the UPDATE announcing `route` on a session. @@ -352,6 +356,20 @@ pub fn build_announce(route: &LocalRoute, towards: &Towards) -> Option Towards { Towards { @@ -401,6 +455,7 @@ mod tests { next_hop, four_octet_asn, ibgp: false, + role: None, } } @@ -675,6 +730,7 @@ mod tests { families: FamilySet::supported(), add_path_receive: FamilySet::default(), add_path_send: FamilySet::default(), + role_confirmed: false, }; let rib_peer = self .table diff --git a/crates/server/src/config.rs b/crates/server/src/config.rs index 6e57196..ceffd57 100644 --- a/crates/server/src/config.rs +++ b/crates/server/src/config.rs @@ -66,6 +66,9 @@ pub struct PeerConfig { pub import: ImportPolicy, /// What the peer is to us, if that was said. pub role: Option, + /// Refuse the peer if it does not say what it is itself (RFC 9234 + /// §4.2, "strict mode"). + pub require_role: bool, } impl Config { @@ -109,6 +112,10 @@ impl Config { problem, }; 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)); + } // A peer is told from the others by its name in what the // dialer reports, and by its address when it connects to us. if peers.iter().any(|earlier| earlier.name == peer.name) { @@ -236,6 +243,9 @@ fn parse_peer(peer: Peer) -> Result { Ok(proto::Role::ROUTE_SERVER_CLIENT) => Some(PeerRole::RouteServerClient), Err(_) => return Err(PeerProblem::Role), }; + if peer.require_role && role.is_none() { + return Err(PeerProblem::RequireRoleWithoutRole); + } Ok(PeerConfig { // A peer without a name goes by its address. @@ -252,6 +262,7 @@ fn parse_peer(peer: Peer) -> Result { add_path, import, role, + require_role: peer.require_role, }) } @@ -764,6 +775,26 @@ mod tests { role_of("role: ROUTE_SERVER").unwrap(), Some(PeerRole::RouteServer) ); + // Roles are between networks, and a peer can be required to say + // its own only where we know what to expect. + let problem = |body: &str| match role_of(body).err().unwrap() { + ConfigError::Peer { problem, .. } => problem, + other => panic!("{other:?}"), + }; + assert_eq!( + problem("role: PEER remote_asn: 65000"), + PeerProblem::RoleInOwnAs + ); + assert_eq!( + problem("require_role: true"), + PeerProblem::RequireRoleWithoutRole + ); + let config = parse(&format!( + "{BASE}peers {{ endpoint {{ address: \"192.0.2.2\" }} remote_asn: 1 \ + role: PEER require_role: true }}" + )) + .unwrap(); + assert!(config.peers[0].require_role); assert!(!import_of("").unwrap().accept_aspa_invalid); assert!( import_of("import { accept_aspa_invalid: true }") diff --git a/crates/server/src/connection.rs b/crates/server/src/connection.rs index 18f73dc..c65c123 100644 --- a/crates/server/src/connection.rs +++ b/crates/server/src/connection.rs @@ -99,6 +99,8 @@ pub struct PeerSettings { pub import: Arc, /// What the peer is to us, if that is known. pub role: Option, + /// Refuse the peer if it does not say what it is (RFC 9234). + pub require_role: bool, } impl PeerSettings { @@ -114,6 +116,7 @@ impl PeerSettings { add_path: None, import: Arc::new(ImportPolicy::permissive()), role: None, + require_role: false, } } } @@ -128,6 +131,7 @@ impl From<&PeerConfig> for PeerSettings { add_path: peer.add_path, import: Arc::new(peer.import.clone()), role: peer.role, + require_role: peer.require_role, } } } @@ -395,6 +399,8 @@ impl Session { expected_asn: peer.expected_asn, families: peer.families, add_path: peer.add_path, + role: peer.role, + require_role: peer.require_role, }); // The peer goes by its configured name, or by its address. let label = peer @@ -675,6 +681,7 @@ impl Session { outbound_tx, ); handle.role = self.role; + handle.role_confirmed = negotiated.role_confirmed; // Routes from other networks are validated, where there is data // to validate against and the peer's policy does not say to take // invalid ones anyway. @@ -708,6 +715,7 @@ impl Session { local_asn: self.server.local_asn, ibgp: negotiated.ibgp, four_octet_asn: negotiated.four_octet_asn, + role: self.role, }, counters: Arc::clone(&handle.counters), routes_metric: PEER_ROUTES.with_label_values(&[self.refusals.label()]), @@ -1188,6 +1196,7 @@ mod tests { local_asn: 65000, ibgp: false, four_octet_asn: true, + role: None, }, policy: &ImportPolicy::permissive(), checks: Checks::NONE, diff --git a/crates/server/src/error.rs b/crates/server/src/error.rs index c08bbc6..c6be0a2 100644 --- a/crates/server/src/error.rs +++ b/crates/server/src/error.rs @@ -6,6 +6,7 @@ use std::time::Duration; use bgp_packet::errors::{BgpError, CeaseSubcode, DecodeError, FsmSubcode, OpenMsgSubcode}; use bgp_packet::message::NotificationMessage; +use bgp_packet::open::BgpRole; use bgp_primitives::parser::EncodeError; use bgp_rib::RibStopped; use bgp_rib::table::TooManyPeers; @@ -13,6 +14,21 @@ use thiserror::Error; use crate::family::FamilySet; use crate::fsm::State; +use crate::policy::PeerRole; + +/// Says what is wrong between what a peer is configured as and what it +/// says it is. +fn role_mismatch(configured: Option, theirs: Option) -> String { + let configured = configured.map_or("no role is configured".to_string(), |role| { + format!("the peer is configured as our {}", role.as_str()) + }); + match theirs { + Some(theirs) => format!("role mismatch: {configured} and says it is {theirs:?}"), + None => format!( + "role mismatch: {configured} and does not say what it is, or says more than one thing" + ), + } +} /// Why a session ended abnormally. #[derive(Debug, Error)] @@ -40,6 +56,13 @@ pub enum SessionError { OpenRefused(BgpError), #[error("peer announced AS{actual} but AS{expected} was configured")] WrongPeerAs { expected: u32, actual: u32 }, + /// RFC 9234 §4.2. `theirs` is `None` if the peer did not say, or said + /// more than one thing. + #[error("{}", role_mismatch(*.configured, *.theirs))] + RoleMismatch { + configured: Option, + theirs: Option, + }, #[error("closed by connection collision resolution")] Collision, /// The routing table has no room for another peer. @@ -88,6 +111,7 @@ impl SessionError { }), Self::OpenRefused(error) => error.clone(), Self::WrongPeerAs { .. } => BgpError::OpenMsg(OpenMsgSubcode::BadPeerAs), + Self::RoleMismatch { .. } => BgpError::OpenMsg(OpenMsgSubcode::RoleMismatch), Self::Collision => BgpError::Cease(CeaseSubcode::ConnCollisionResolution), Self::TableFull(_) => BgpError::Cease(CeaseSubcode::OutOfResources), Self::PrefixLimitExceeded { .. } => BgpError::Cease(CeaseSubcode::MaxPrefixes), @@ -179,6 +203,10 @@ pub enum PeerProblem { AddressFamily, #[error("add_path has a value that is not one of its settings")] AddPath, + #[error("role is set for a peer in our own AS; roles are between networks")] + RoleInOwnAs, + #[error("require_role is set without a role")] + RequireRoleWithoutRole, #[error("role has a value that is not one of its settings")] Role, #[error("import: {0}")] diff --git a/crates/server/src/fsm.rs b/crates/server/src/fsm.rs index 5b06ebd..d5dcd9c 100644 --- a/crates/server/src/fsm.rs +++ b/crates/server/src/fsm.rs @@ -21,6 +21,7 @@ use bgp_packet::open::{AddPathEntry, AddPathSendRecv, Capability, OpenMessage, O use crate::error::SessionError; use crate::family::FamilySet; use crate::metrics::message_type_name; +use crate::policy::PeerRole; /// How long to wait for the peer's OPEN, and for its first KEEPALIVE when /// the negotiated hold time is zero. RFC 4271 §8.2.2 suggests four minutes. @@ -65,6 +66,13 @@ pub struct FsmConfig { /// The directions to offer ADD-PATH (RFC 7911) for, on each of /// `families`; `None` for neither. pub add_path: Option, + /// What the peer is to us, if that is configured. Our OPEN then says + /// what we are to it, and the peer's must not say otherwise (RFC 9234). + /// Not for a peer in our own AS. + pub role: Option, + /// Refuse a peer that does not say what it is (RFC 9234 §4.2, "strict + /// mode"). Means nothing without `role`. + pub require_role: bool, } /// What the two OPENs settled. @@ -90,6 +98,9 @@ pub struct Negotiated { /// The families for which we send the peer path identifiers: we /// offered to send and the peer to receive. pub add_path_send: FamilySet, + /// Whether the peer said what it is to us, and it is what we have it + /// configured as (RFC 9234). + pub role_confirmed: bool, } /// A message from the peer, with what the decoder found out besides. @@ -309,6 +320,36 @@ impl Fsm { }); } + // RFC 9234 §4.2: the two ends have to agree on what they are to + // each other. + let mut their_roles = open + .capabilities() + .filter_map(|capability| match capability { + Capability::BgpRole { role } => Some(*role), + _ => None, + }); + let their_role = their_roles.next(); + let role_mismatch = |theirs| SessionError::RoleMismatch { + configured: self.config.role, + theirs, + }; + // Saying it twice is saying it once; saying two things is saying + // nothing that can be relied on. + if their_roles.any(|role| Some(role) != their_role) { + return self.fail(role_mismatch(None)); + } + let role_confirmed = match (self.config.role, their_role) { + (Some(configured), Some(theirs)) if theirs == configured.theirs() => true, + (Some(_), Some(theirs)) => return self.fail(role_mismatch(Some(theirs))), + (Some(_), None) if self.config.require_role => { + return self.fail(role_mismatch(None)); + } + // A peer that does not say is taken at our word for it; and + // what a peer says of itself means nothing where nothing is + // configured to hold it against. + (Some(_), None) | (None, _) => false, + }; + let ours = self.config.families.clone(); let theirs = FamilySet::offered_by(&open); let families = ours.intersection(&theirs); @@ -348,6 +389,7 @@ impl Fsm { families, add_path_receive, add_path_send, + role_confirmed, }; // With a hold time of zero the timer will not run once Established, // but a peer that never confirms must still be given up on. @@ -419,6 +461,9 @@ fn build_open(config: &FsmConfig) -> Message { // not accept routes of a family that was not offered (RFC 4760 §8). capabilities.extend(config.families.iter().map(Capability::multiprotocol)); capabilities.push(Capability::RouteRefresh); + if let Some(role) = config.role { + capabilities.push(Capability::BgpRole { role: role.ours() }); + } if let Some(send_recv) = config.add_path { capabilities.push(Capability::AddPath { entries: config @@ -468,6 +513,8 @@ mod tests { use bgp_rib::table::TooManyPeers; + use bgp_packet::open::BgpRole; + use super::*; use crate::family::SUPPORTED; @@ -482,6 +529,8 @@ mod tests { expected_asn: None, families: FamilySet::supported(), add_path: None, + role: None, + require_role: false, } } @@ -582,6 +631,7 @@ mod tests { families: only(AfiSafi::IPV4_UNICAST), add_path_receive: FamilySet::default(), add_path_send: FamilySet::default(), + role_confirmed: false, }; assert!(matches!( &actions[..], @@ -812,6 +862,152 @@ mod tests { )); } + /// What becomes of a session with a peer configured as `role`, whose + /// OPEN carries the Role capabilities `theirs`: whether the peer + /// confirmed its role, or the error the session ended with. + fn roles_meet( + role: Option, + require_role: bool, + theirs: &[BgpRole], + ) -> Result { + let (mut fsm, _) = Fsm::new(FsmConfig { + role, + require_role, + ..config() + }); + let open = OpenMessage { + options: vec![OpenOption::Capabilities( + theirs + .iter() + .map(|role| Capability::BgpRole { role: *role }) + .collect(), + )], + ..peer_open(90) + }; + let actions = fsm.handle(received(Message::Open(open))); + match &actions[..] { + [Action::Admit(negotiated), ..] => Ok(negotiated.role_confirmed), + _ => Err(ending(&fsm, actions).0.expect("a NOTIFICATION")), + } + } + + /// RFC 9234 §4.1: our OPEN says what we are to the peer, which is the + /// other half of what the peer is to us. + #[test] + fn our_open_says_what_we_are_to_the_peer() { + for (peer_is, we_are) in [ + (PeerRole::Provider, BgpRole::Customer), + (PeerRole::Customer, BgpRole::Provider), + (PeerRole::Peer, BgpRole::Peer), + (PeerRole::RouteServer, BgpRole::RouteServerClient), + (PeerRole::RouteServerClient, BgpRole::RouteServer), + ] { + let (_, actions) = Fsm::new(FsmConfig { + role: Some(peer_is), + ..config() + }); + let Action::Send(open) = &actions[0] else { + panic!("expected an OPEN first, got {actions:?}"); + }; + let roles: Vec<_> = capabilities(open) + .into_iter() + .filter(|capability| matches!(capability, Capability::BgpRole { .. })) + .collect(); + assert_eq!( + roles, + [&Capability::BgpRole { role: we_are }], + "{peer_is:?}" + ); + } + // Nothing is said where nothing is configured. + let (_, actions) = Fsm::new(config()); + let Action::Send(open) = &actions[0] else { + panic!("expected an OPEN first, got {actions:?}"); + }; + assert!( + !capabilities(open) + .iter() + .any(|capability| matches!(capability, Capability::BgpRole { .. })) + ); + } + + /// RFC 9234 §4.2. + #[test] + fn a_peer_that_says_it_is_something_else_than_configured_is_refused() { + let mismatch = Err(BgpError::OpenMsg(OpenMsgSubcode::RoleMismatch)); + let all = [ + BgpRole::Provider, + BgpRole::RouteServer, + BgpRole::RouteServerClient, + BgpRole::Customer, + BgpRole::Peer, + ]; + for configured in [ + PeerRole::Provider, + PeerRole::Customer, + PeerRole::Peer, + PeerRole::RouteServer, + PeerRole::RouteServerClient, + ] { + for theirs in all { + let expected = if theirs == configured.theirs() { + Ok(true) + } else { + mismatch.clone() + }; + assert_eq!( + roles_meet(Some(configured), false, &[theirs]), + expected, + "configured as {configured:?}, says {theirs:?}" + ); + } + // A role nobody has defined is not the one configured. + assert_eq!( + roles_meet(Some(configured), false, &[BgpRole::Unknown(9)]), + mismatch + ); + } + } + + #[test] + fn a_peer_that_does_not_say_what_it_is_is_refused_only_if_it_must() { + let mismatch = Err(BgpError::OpenMsg(OpenMsgSubcode::RoleMismatch)); + // A peer that knows nothing of roles is taken at our word for it, + assert_eq!(roles_meet(Some(PeerRole::Customer), false, &[]), Ok(false)); + // unless it is required to say. + assert_eq!(roles_meet(Some(PeerRole::Customer), true, &[]), mismatch); + assert_eq!( + roles_meet(Some(PeerRole::Customer), true, &[BgpRole::Customer]), + Ok(true) + ); + // What a peer says where nothing is configured confirms nothing. + assert_eq!(roles_meet(None, false, &[BgpRole::Provider]), Ok(false)); + } + + #[test] + fn a_peer_that_says_two_things_is_refused() { + let mismatch = Err(BgpError::OpenMsg(OpenMsgSubcode::RoleMismatch)); + // The same thing twice is the same thing. + assert_eq!( + roles_meet(Some(PeerRole::Peer), false, &[BgpRole::Peer, BgpRole::Peer]), + Ok(true) + ); + // Two things, one of them right, is not. + assert_eq!( + roles_meet( + Some(PeerRole::Peer), + false, + &[BgpRole::Peer, BgpRole::Customer] + ), + mismatch + ); + // Not even where we have nothing configured to compare with. + assert_eq!( + roles_meet(None, false, &[BgpRole::Peer, BgpRole::Customer]), + mismatch + ); + } + #[test] fn an_unacceptable_open_is_refused_with_the_matching_subcode() { let refused = |open: OpenMessage, config: FsmConfig| { diff --git a/crates/server/src/policy.rs b/crates/server/src/policy.rs index 3e38922..e0e993e 100644 --- a/crates/server/src/policy.rs +++ b/crates/server/src/policy.rs @@ -30,6 +30,7 @@ use std::sync::{LazyLock, Mutex}; use std::time::{Duration, Instant}; use bgp_packet::message::{AS_TRANS, AsPathAttribute, PathAttribute}; +use bgp_packet::open::BgpRole; use bgp_primitives::constants::IpVersion; use bgp_primitives::ip_prefix::IpPrefix; @@ -150,10 +151,14 @@ pub enum Rejection { /// A network on the path has said who its providers are, and the path /// does not go by them: a route leak, or a path that was made up. AspaInvalid, + /// The route is marked as one that may only be passed down to + /// customers, and it did not come from above: someone has leaked it + /// (RFC 9234). + Leak, } impl Rejection { - pub const ALL: [Self; 11] = [ + pub const ALL: [Self; 12] = [ Self::BogonPrefix, Self::TooShort, Self::TooSpecific, @@ -165,6 +170,7 @@ impl Rejection { Self::OriginNotAllowed, Self::RpkiInvalid, Self::AspaInvalid, + Self::Leak, ]; /// Where the reason is in [`Rejection::ALL`]. @@ -185,6 +191,7 @@ impl Rejection { Self::OriginNotAllowed => "origin AS not allowed", Self::RpkiInvalid => "RPKI invalid", Self::AspaInvalid => "ASPA invalid", + Self::Leak => "route leak (Only to Customer)", } } } @@ -209,6 +216,57 @@ pub enum PeerRole { } impl PeerRole { + /// The role the peer has to announce in its OPEN: its own (RFC 9234 + /// §4.1). + pub fn theirs(self) -> BgpRole { + match self { + Self::Provider => BgpRole::Provider, + Self::Customer => BgpRole::Customer, + Self::Peer => BgpRole::Peer, + Self::RouteServer => BgpRole::RouteServer, + Self::RouteServerClient => BgpRole::RouteServerClient, + } + } + + /// The role we announce in ours: what we are to the peer, which is + /// the other half of the pair (RFC 9234 §4.2). + pub fn ours(self) -> BgpRole { + match self { + Self::Provider => BgpRole::Customer, + Self::Customer => BgpRole::Provider, + Self::Peer => BgpRole::Peer, + Self::RouteServer => BgpRole::RouteServerClient, + Self::RouteServerClient => BgpRole::RouteServer, + } + } + + /// Whether the peer is above or beside us, and not below: a provider, + /// a peer, or a route server. A route that came from there may only + /// go on to customers. + fn is_above_or_beside(self) -> bool { + matches!(self, Self::Provider | Self::Peer | Self::RouteServer) + } + + /// The Only to Customer attribute to add to a route received from the + /// peer, whose AS number is `peer_asn`, if the route does not have + /// one (RFC 9234 §5, ingress, 3). + pub fn only_to_customer_on_import(self, peer_asn: u32) -> Option { + self.is_above_or_beside().then_some(peer_asn) + } + + /// The Only to Customer attribute to add to a route sent to the peer + /// if the route does not have one (RFC 9234 §5, egress, 1): the route + /// is going down or across, and may not come up again. + pub fn only_to_customer_on_export(self, local_asn: u32) -> Option { + matches!(self, Self::Customer | Self::Peer | Self::RouteServerClient).then_some(local_asn) + } + + /// Whether a route with an Only to Customer attribute may be sent to + /// the peer: not upwards or across (RFC 9234 §5, egress, 2). + pub fn may_be_sent_only_to_customer_routes(self) -> bool { + !self.is_above_or_beside() + } + pub fn as_str(self) -> &'static str { match self { Self::Provider => "provider", @@ -231,6 +289,8 @@ pub struct From { /// Whether AS numbers are four octets wide on the session. If not, a /// path may hold AS_TRANS where a wider number belongs. pub four_octet_asn: bool, + /// What the peer is to us, if that is known. + pub role: Option, } impl ImportPolicy { @@ -265,6 +325,24 @@ impl ImportPolicy { if facts.has_local_asn { return Err(Rejection::OwnAs); } + // RFC 9234 §5, ingress, 1 and 2. A route that is only to go to + // customers has been down or across once already. From a customer + // it is coming up again; from a peer it is coming across again, + // unless it was the peer itself that marked it, on its way to us. + let only_to_customer = attributes.iter().find_map(|attribute| match attribute { + PathAttribute::OnlyToCustomer(asn) => Some(*asn), + _ => None, + }); + if let (Some(marked_by), Some(role)) = (only_to_customer, from.role) { + let leak = match role { + PeerRole::Customer | PeerRole::RouteServerClient => true, + PeerRole::Peer => marked_by != from.peer_asn, + PeerRole::Provider | PeerRole::RouteServer => false, + }; + if leak { + return Err(Rejection::Leak); + } + } if !self.accept_any_first_as { // The first AS of the first segment, which has to be a // sequence: a set has no first. @@ -572,6 +650,7 @@ mod tests { local_asn: 1299, ibgp: false, four_octet_asn: true, + role: None, }; fn path(asns: &[u32]) -> Vec { @@ -809,6 +888,100 @@ mod tests { ); } + /// RFC 9234 §5, the ingress procedure. + #[test] + fn a_route_only_for_customers_is_a_leak_from_below_or_from_a_second_peer() { + let policy = ImportPolicy::default(); + let marked_by = |asn: u32| { + let mut attributes = path(&[3333, 2121]); + attributes.push(PathAttribute::OnlyToCustomer(asn)); + attributes + }; + let from = |role| From { + role: Some(role), + ..EXTERNAL + }; + // From below it is always a leak. + for role in [PeerRole::Customer, PeerRole::RouteServerClient] { + for asn in [3333, 2121] { + assert_eq!( + policy.check_path(&marked_by(asn), &from(role)), + Err(Rejection::Leak), + "{role:?}" + ); + } + } + // From a peer, unless the peer marked it itself. + assert_eq!( + policy.check_path(&marked_by(2121), &from(PeerRole::Peer)), + Err(Rejection::Leak) + ); + assert_eq!( + policy.check_path(&marked_by(3333), &from(PeerRole::Peer)), + Ok(Some(2121)) + ); + // From above it is where it should be. + for role in [PeerRole::Provider, PeerRole::RouteServer] { + assert_eq!( + policy.check_path(&marked_by(2121), &from(role)), + Ok(Some(2121)), + "{role:?}" + ); + } + // Without the mark nothing is known, whoever it is from; and + // without knowing what the peer is, the mark says nothing. + for role in [PeerRole::Customer, PeerRole::Peer, PeerRole::Provider] { + assert_eq!( + policy.check_path(&path(&[3333, 2121]), &from(role)), + Ok(Some(2121)) + ); + } + assert_eq!( + policy.check_path(&marked_by(2121), &EXTERNAL), + Ok(Some(2121)) + ); + // Not even a policy that takes everything else takes a leak. + assert_eq!( + ImportPolicy::permissive().check_path(&marked_by(2121), &from(PeerRole::Customer)), + Err(Rejection::Leak) + ); + } + + #[test] + fn roles_pair_up_and_decide_where_the_mark_is_put() { + use PeerRole::*; + for role in [Provider, Customer, Peer, RouteServer, RouteServerClient] { + // What we are to the peer is what the peer is to us, seen + // from the other side. + let mirrored = [Provider, Customer, Peer, RouteServer, RouteServerClient] + .into_iter() + .find(|other| other.theirs() == role.ours()) + .unwrap(); + assert_eq!(mirrored.ours(), role.theirs(), "{role:?}"); + } + assert_eq!(Provider.ours(), BgpRole::Customer); + assert_eq!(RouteServer.ours(), BgpRole::RouteServerClient); + assert_eq!(Peer.ours(), BgpRole::Peer); + + // Marked on the way in from above or beside; on the way out + // downwards or across. + assert_eq!(Provider.only_to_customer_on_import(3333), Some(3333)); + assert_eq!(Peer.only_to_customer_on_import(3333), Some(3333)); + assert_eq!(RouteServer.only_to_customer_on_import(3333), Some(3333)); + assert_eq!(Customer.only_to_customer_on_import(3333), None); + assert_eq!(RouteServerClient.only_to_customer_on_import(3333), None); + assert_eq!(Customer.only_to_customer_on_export(1299), Some(1299)); + assert_eq!(Peer.only_to_customer_on_export(1299), Some(1299)); + assert_eq!( + RouteServerClient.only_to_customer_on_export(1299), + Some(1299) + ); + assert_eq!(Provider.only_to_customer_on_export(1299), None); + assert_eq!(RouteServer.only_to_customer_on_export(1299), None); + assert!(Customer.may_be_sent_only_to_customer_routes()); + assert!(!Peer.may_be_sent_only_to_customer_routes()); + } + #[test] fn the_path_is_read_with_its_as4_path_and_its_sets() { let mut out = vec![99]; diff --git a/crates/server/src/routes.rs b/crates/server/src/routes.rs index 62aae7e..ad08dfd 100644 --- a/crates/server/src/routes.rs +++ b/crates/server/src/routes.rs @@ -6,7 +6,7 @@ use std::cell::OnceCell; use std::collections::HashMap; use std::sync::Arc; -use bgp_packet::message::{NlriNextHop, PathAttribute, UpdateMessage}; +use bgp_packet::message::{NlriNextHop, PathAttribute, UpdateMessage, only_to_customer_wire}; use bgp_primitives::ip_prefix::Nlri; use bgp_primitives::parser::EncodeError; use bgp_rib::table::{AttrSet, Outcome, PeerIndex, RibTable}; @@ -138,13 +138,39 @@ pub(crate) fn apply( // as far as that mattering. let path_invalid = OnceCell::new(); + // A route from above or beside us may from here on only go down to + // customers. If nobody has marked it so yet, we do (RFC 9234 §5, + // ingress, 3): the mark is part of the route as it is kept, and goes + // wherever the route goes. + let mark = import + .from + .role + .filter(|_| !import.from.ibgp) + .and_then(|role| role.only_to_customer_on_import(import.from.peer_asn)) + .filter(|_| { + !update + .path_attributes + .iter() + .any(|attribute| matches!(attribute, PathAttribute::OnlyToCustomer(_))) + }); let attributes = |next_hop: Option<&NlriNextHop>| -> Result, EncodeError> { - if let Some(set) = - attributes_wire.and_then(|wire| table.attrs().intern_received(wire, next_hop)) + let Some(marked_by) = mark else { + if let Some(set) = + attributes_wire.and_then(|wire| table.attrs().intern_received(wire, next_hop)) + { + return Ok(set); + } + return table.attrs().intern(&update.path_attributes, next_hop); + }; + let added = only_to_customer_wire(marked_by); + if let Some(set) = attributes_wire + .and_then(|wire| table.attrs().intern_received_with(wire, &added, next_hop)) { return Ok(set); } - table.attrs().intern(&update.path_attributes, next_hop) + let mut marked = update.path_attributes.clone(); + marked.push(PathAttribute::OnlyToCustomer(marked_by)); + table.attrs().intern(&marked, next_hop) }; let mut announce = |nlri: &[Nlri], next_hop| -> Result<(), EncodeError> { if nlri.is_empty() { diff --git a/crates/server/src/session.rs b/crates/server/src/session.rs index a6d6274..8bc7da8 100644 --- a/crates/server/src/session.rs +++ b/crates/server/src/session.rs @@ -123,6 +123,8 @@ pub struct SessionHandle { pub ibgp: bool, /// What the peer is to us, if that is known. pub role: Option, + /// Whether the peer said so itself in its OPEN (RFC 9234). + pub role_confirmed: bool, /// Whether the peer negotiated RFC 6793 four-octet ASNs. Determines how /// AS_PATH is encoded on the wire for this session. pub four_octet_asn: bool, @@ -167,6 +169,7 @@ impl SessionHandle { established_at: SystemTime::now(), ibgp: negotiated.ibgp, role: None, + role_confirmed: false, four_octet_asn: negotiated.four_octet_asn, families: negotiated.families.clone(), add_path_receive: negotiated.add_path_receive.clone(), diff --git a/crates/web/src/api.rs b/crates/web/src/api.rs index ca0ab69..f2fd0b1 100644 --- a/crates/web/src/api.rs +++ b/crates/web/src/api.rs @@ -82,6 +82,9 @@ pub struct PeerView { /// What the peer is to us, if configured: `provider`, `customer`, /// `peer`, `route server` or `route server client`. pub role: Option<&'static str>, + /// 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, /// Why the last connection attempt or session ended, while in Backoff. pub last_error: Option, pub retry_at_unix: Option, @@ -106,6 +109,7 @@ impl From<&SessionHandle> for PeerView { rejected_routes: Some(s.counters.rejected()), held_routes: Some(s.counters.held()), role: s.role.map(PeerRole::as_str), + role_confirmed: s.role.map(|_| s.role_confirmed), last_error: None, retry_at_unix: None, } @@ -138,6 +142,7 @@ impl PeerView { rejected_routes: None, held_routes: None, role: peer.role.map(PeerRole::as_str), + role_confirmed: None, last_error, retry_at_unix, } diff --git a/crates/web/static/statusz.html b/crates/web/static/statusz.html index b614e78..dad9633 100644 --- a/crates/web/static/statusz.html +++ b/crates/web/static/statusz.html @@ -175,7 +175,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)'}${esc(p.peer_addr)}${p.peer_asn}${p.role ? ` ${esc(p.role)}` : ''} + ${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/docs/RFCs.md b/docs/RFCs.md index 1695643..5a4ec3a 100644 --- a/docs/RFCs.md +++ b/docs/RFCs.md @@ -201,7 +201,20 @@ Implemented from version 24 of the draft. | Routes revalidated when the ASPAs change, in both directions | ✅ | | Paths with an AS_SET invalid | ✅ | | Verification on export | ❌ (learned routes are not exported yet) | -| Roles negotiated in OPEN, Only-to-Customer attribute (RFC 9234) | ❌ | + +### RFC 9234 — Route Leak Prevention and Detection Using Roles in UPDATE and OPEN Messages +https://datatracker.ietf.org/doc/html/rfc9234 + +| Feature | Status | +|---------|--------| +| BGP Role capability sent for peers with a configured `role` | ✅ | +| Role Mismatch (OPEN Message Error, subcode 11) when the peer's role does not pair with ours, or it sends differing roles | ✅ | +| Strict mode (`require_role`) | ✅ | +| Only to Customer attribute: parse, serialise, treat-as-withdraw when malformed | ✅ | +| Ingress: marked routes from customers, route server clients and (unless marked by the peer itself) peers refused | ✅ | +| Ingress: routes from providers, peers and route servers marked | ✅ | +| Egress: originated routes to customers, peers and route server clients marked | ✅ | +| Egress: marked routes not sent to providers, peers or route servers | ❌ (learned routes are not exported yet) | ### RFC 8210 — The RPKI to Router Protocol, Version 1 (RFC 6810, version 0; draft-ietf-sidrops-8210bis, version 2) https://datatracker.ietf.org/doc/html/rfc8210 diff --git a/testdata/server.textpb b/testdata/server.textpb index 61ecf24..bfa351a 100644 --- a/testdata/server.textpb +++ b/testdata/server.textpb @@ -23,9 +23,13 @@ peers { # them (ADD-PATH). Also ADD_PATH_SEND and ADD_PATH_BOTH. Off by default. add_path: ADD_PATH_RECEIVE # What the peer is to us: PROVIDER, CUSTOMER, PEER, ROUTE_SERVER or - # ROUTE_SERVER_CLIENT. With it, and a validator that has ASPAs, the AS - # paths of the peer's routes are verified, and leaked routes refused. + # ROUTE_SERVER_CLIENT. With it the peer must not claim to be something + # else (RFC 9234), routes are marked "only to customer" and leaks of + # marked routes are refused; and with a validator that has ASPAs, the + # AS paths of the peer's routes are verified. # role: PEER + # Refuse the peer if its OPEN does not say what it is. + # require_role: true # Which of the peer's routes are taken. Without this block the general # checks still apply: no private or reserved prefixes or AS numbers, # nothing longer than /24 or /48, no default route, and a path that