From 22d0099a46a4e655a5e52fb3d4ec122dfdb72854 Mon Sep 17 00:00:00 2001 From: Rayhaan Jaufeerally Date: Thu, 1 Oct 2026 22:22:54 +0200 Subject: [PATCH] Negotiate address families and AS number width instead of assuming them The OPEN used to offer IPv4 and IPv6 unicast unconditionally, and what the peer offered was ignored apart from its four-octet AS number. Address families (RFC 4760): - A peer's `address_families` in the configuration says which to offer it; all of them by default. - A session carries the families both OPENs offered. A peer that sends no Multiprotocol capability is a plain RFC 4271 speaker: IPv4 unicast. - Routes of a family that was not negotiated are not sent, and are ignored when received; the rest of the UPDATE is applied and the session stays up. - A session with no family in common is refused with Unsupported Capability, naming the capabilities we needed (RFC 5492). - The negotiated families are shown on /statusz and in /api/peers. Bugs found on the way: - A capability naming an address family we have no constant for (EVPN, VPNv4, ...) failed the whole OPEN, so such a peer could never establish. It is now kept as an unknown capability and ignored. - An UPDATE could not be encoded for a peer without four-octet AS support, which ended the session on the first route. AS_PATH is now written two octets wide with AS_TRANS where needed, and AS4_PATH carries the real numbers (RFC 6793). Co-Authored-By: Claude Fable 5.1 --- crates/integration-tests/src/test_peer.rs | 71 +++- crates/integration-tests/tests/collision.rs | 2 + crates/integration-tests/tests/dialer.rs | 5 + crates/integration-tests/tests/http_api.rs | 4 + crates/integration-tests/tests/interop.rs | 2 +- crates/integration-tests/tests/negotiation.rs | 314 +++++++++++++++ crates/packet/src/message.rs | 211 ++++++++--- crates/packet/src/open.rs | 191 +++++++--- crates/primitives/src/parser.rs | 4 + crates/server/proto/config.proto | 13 + crates/server/src/advertise.rs | 97 ++++- crates/server/src/config.rs | 47 ++- crates/server/src/connection.rs | 14 + crates/server/src/dialer.rs | 1 + crates/server/src/error.rs | 15 + crates/server/src/family.rs | 357 ++++++++++++++++++ crates/server/src/fsm.rs | 176 +++++++-- crates/server/src/lib.rs | 1 + crates/server/src/server.rs | 5 +- crates/server/src/session.rs | 6 + crates/web/src/api.rs | 4 + crates/web/static/statusz.html | 7 +- testdata/server.textpb | 3 + 23 files changed, 1391 insertions(+), 159 deletions(-) create mode 100644 crates/integration-tests/tests/negotiation.rs create mode 100644 crates/server/src/family.rs diff --git a/crates/integration-tests/src/test_peer.rs b/crates/integration-tests/src/test_peer.rs index e107151..b7d9cc1 100644 --- a/crates/integration-tests/src/test_peer.rs +++ b/crates/integration-tests/src/test_peer.rs @@ -14,6 +14,9 @@ use tokio::net::TcpStream; pub struct TestPeer { stream: TcpStream, pub ctx: ParserContext, + /// Whether the OPEN we sent offered four-octet AS numbers. They are in + /// use only if the server's did too. + offered_four_octet_asn: bool, } impl TestPeer { @@ -28,6 +31,7 @@ impl TestPeer { Self { stream, ctx: ParserContext::default(), + offered_four_octet_asn: true, } } @@ -81,20 +85,52 @@ impl TestPeer { matches!(opt, OpenOption::Capabilities(caps) if caps.iter().any(|c| matches!(c, Capability::FourByteAsn { .. }))) }); - self.ctx.four_octet_asn = Some(four_byte); + self.ctx.four_octet_asn = Some(four_byte && self.offered_four_octet_asn); Ok(o) } other => bail!("Expected OPEN, got {:?}", other), } } - /// Send an OPEN message with the given parameters and standard capabilities. + /// Send an OPEN message with the given parameters and the capabilities + /// of a typical dual-stack speaker: four-octet AS numbers, IPv4 and IPv6 + /// unicast, route refresh. pub async fn send_open( &mut self, local_asn: u32, router_id: Ipv4Addr, hold_time: u16, ) -> Result<()> { + let capabilities = vec![ + Capability::FourByteAsn { asn: local_asn }, + Capability::MultiProtocol { + afi: AddressFamilyId::Ipv4, + safi: SubsequentAfi::Unicast, + }, + Capability::MultiProtocol { + afi: AddressFamilyId::Ipv6, + safi: SubsequentAfi::Unicast, + }, + Capability::RouteRefresh, + ]; + self.send_open_with(local_asn, router_id, hold_time, capabilities) + .await + } + + /// Send an OPEN message carrying exactly `capabilities`. + pub async fn send_open_with( + &mut self, + local_asn: u32, + router_id: Ipv4Addr, + hold_time: u16, + capabilities: Vec, + ) -> Result<()> { + self.offered_four_octet_asn = capabilities + .iter() + .any(|c| matches!(c, Capability::FourByteAsn { .. })); + if let Some(negotiated) = &mut self.ctx.four_octet_asn { + *negotiated &= self.offered_four_octet_asn; + } let asn_field = if local_asn > u16::MAX as u32 { 23456 } else { @@ -105,14 +141,11 @@ impl TestPeer { asn: asn_field, hold_time, identifier: router_id, - options: vec![OpenOption::Capabilities(vec![ - Capability::FourByteAsn { asn: local_asn }, - Capability::MultiProtocol { - afi: AddressFamilyId::Ipv4, - safi: SubsequentAfi::Unicast, - }, - Capability::RouteRefresh, - ])], + options: if capabilities.is_empty() { + vec![] + } else { + vec![OpenOption::Capabilities(capabilities)] + }, }); self.send(&open).await } @@ -126,6 +159,24 @@ impl TestPeer { hold_time: u16, ) -> Result { self.send_open(local_asn, router_id, hold_time).await?; + self.finish_handshake().await + } + + /// As [`TestPeer::handshake`], with an OPEN carrying exactly + /// `capabilities`. + pub async fn handshake_with( + &mut self, + local_asn: u32, + router_id: Ipv4Addr, + hold_time: u16, + capabilities: Vec, + ) -> Result { + self.send_open_with(local_asn, router_id, hold_time, capabilities) + .await?; + self.finish_handshake().await + } + + async fn finish_handshake(&mut self) -> Result { let server_open = self.recv_open().await?; // Expect server KEEPALIVE match self.recv().await? { diff --git a/crates/integration-tests/tests/collision.rs b/crates/integration-tests/tests/collision.rs index 5e8165b..56edcb9 100644 --- a/crates/integration-tests/tests/collision.rs +++ b/crates/integration-tests/tests/collision.rs @@ -17,6 +17,7 @@ use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use bgp_server::collision::Direction; use bgp_server::config::PeerConfig; +use bgp_server::family::FamilySet; use eyre::Result; use tokio::net::TcpListener; @@ -39,6 +40,7 @@ async fn server_dialing(listener: &TcpListener) -> Result { remote_asn: PEER_ASN, hold_time_secs: 90, local_address: None, + families: FamilySet::ALL, }]; }) .await diff --git a/crates/integration-tests/tests/dialer.rs b/crates/integration-tests/tests/dialer.rs index 6d8f1d2..8b76709 100644 --- a/crates/integration-tests/tests/dialer.rs +++ b/crates/integration-tests/tests/dialer.rs @@ -11,6 +11,7 @@ use bgp_packet::message::Message; use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use bgp_server::config::PeerConfig; +use bgp_server::family::FamilySet; use bgp_server::session::DialState; use eyre::Result; use tokio::net::TcpListener; @@ -30,6 +31,7 @@ async fn server_dialing(listener: &TcpListener, remote_asn: u32) -> Result Result<()> { remote_asn: PEER_ASN, hold_time_secs: 90, local_address: None, + families: FamilySet::ALL, }]; }) .await?; @@ -228,6 +231,7 @@ async fn test_local_address_is_used_as_the_source() -> Result<()> { remote_asn: PEER_ASN, hold_time_secs: 90, local_address: Some(Ipv4Addr::LOCALHOST.into()), + families: FamilySet::ALL, }]; }) .await?; @@ -243,6 +247,7 @@ async fn test_local_address_is_used_as_the_source() -> Result<()> { remote_asn: PEER_ASN, hold_time_secs: 90, local_address: Some(Ipv4Addr::new(192, 0, 2, 1).into()), + families: FamilySet::ALL, }]; }) .await?; diff --git a/crates/integration-tests/tests/http_api.rs b/crates/integration-tests/tests/http_api.rs index 81513be..8f839f9 100644 --- a/crates/integration-tests/tests/http_api.rs +++ b/crates/integration-tests/tests/http_api.rs @@ -72,6 +72,10 @@ async fn test_status_and_peers() -> Result<()> { assert_eq!(peers[0]["peer_asn"], PEER_ASN); assert_eq!(peers[0]["local_addr"], server.addr.to_string()); assert_eq!(peers[0]["four_octet_asn"], true); + assert_eq!( + peers[0]["address_families"], + serde_json::json!(["IPv4 unicast", "IPv6 unicast"]) + ); Ok(()) } diff --git a/crates/integration-tests/tests/interop.rs b/crates/integration-tests/tests/interop.rs index 3b252b3..da41343 100644 --- a/crates/integration-tests/tests/interop.rs +++ b/crates/integration-tests/tests/interop.rs @@ -119,7 +119,7 @@ async fn test_ipv6_prefix_update_shape() -> Result<()> { LocalRouteAttrs::default(), ); let nh: std::net::Ipv6Addr = "fd00:179::2".parse()?; - let update = bgp_server::advertise::build_announce(&route, 4200000001, nh.into()) + let update = bgp_server::advertise::build_announce(&route, 4200000001, nh.into(), true) .expect("ipv6 over ipv6"); assert!(update.announced_routes.is_empty()); assert!( diff --git a/crates/integration-tests/tests/negotiation.rs b/crates/integration-tests/tests/negotiation.rs new file mode 100644 index 0000000..68325be --- /dev/null +++ b/crates/integration-tests/tests/negotiation.rs @@ -0,0 +1,314 @@ +//! End-to-end tests for capability negotiation: what the two OPENs agree on +//! decides which routes are sent and accepted, and how they are encoded. + +use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr}; +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::Capability; +use bgp_primitives::constants::{AddressFamilyId, SubsequentAfi}; +use bgp_primitives::ip_prefix::IpPrefix; +use bgp_rib::{LocalRoute, LocalRouteAttrs}; +use bgp_server::config::PeerConfig; +use bgp_server::family::{AddressFamily, FamilySet}; +use eyre::{Result, bail}; + +const LOCAL_ASN: u32 = 65000; +const PEER_ASN: u32 = 65001; +const PEER_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 1); +const TIMEOUT: Duration = Duration::from_secs(5); + +fn prefix(s: &str) -> IpPrefix { + IpPrefix::try_from(s).unwrap() +} + +fn only(family: AddressFamily) -> FamilySet { + [family].into_iter().collect() +} + +fn four_octet() -> Capability { + Capability::FourByteAsn { asn: PEER_ASN } +} + +async fn wait_for_session(server: &TestServer) -> Result { + Ok(tokio::time::timeout(TIMEOUT, async { + loop { + if let Some(session) = server.registry.list().into_iter().next() { + return session; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await?) +} + +/// An UPDATE announcing one IPv4 route in the body and one IPv6 route in +/// MP_REACH_NLRI. +fn dual_stack_update() -> UpdateMessage { + UpdateMessage { + withdrawn_routes: vec![], + path_attributes: vec![ + PathAttribute::Origin(OriginPathAttribute::IGP), + AsPathAttribute::from_asns(vec![PEER_ASN]), + PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), + PathAttribute::MpReachNlri(MpReachNlriPathAttribute { + afi: AddressFamilyId::Ipv6, + safi: SubsequentAfi::Unicast, + next_hop: NlriNextHop::Ipv6(Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1)), + prefixes: vec![prefix("2001:db8:1::/48")], + }), + ], + announced_routes: vec![prefix("192.0.2.0/24")], + } +} + +/// The session carries the families both OPENs offered, and the server's +/// OPEN offers every family by default. +#[tokio::test] +async fn test_families_are_the_ones_both_sides_offered() -> Result<()> { + let server = TestServer::start(LOCAL_ASN).await?; + let mut peer = TestPeer::connect(server.addr).await?; + let open = peer + .handshake_with( + PEER_ASN, + PEER_ID, + 90, + vec![four_octet(), AddressFamily::Ipv4Unicast.capability()], + ) + .await?; + + for family in AddressFamily::ALL { + assert!( + open.capabilities().any(|c| *c == family.capability()), + "the server's OPEN offers {family}" + ); + } + let session = wait_for_session(&server).await?; + assert_eq!(session.families, only(AddressFamily::Ipv4Unicast)); + Ok(()) +} + +/// A route of a family the peer did not offer is not accepted from it. The +/// rest of the UPDATE is, and the session stays up. +#[tokio::test] +async fn test_routes_of_a_family_not_negotiated_are_not_accepted() -> Result<()> { + let server = TestServer::start(LOCAL_ASN).await?; + let mut peer = TestPeer::connect(server.addr).await?; + peer.handshake_with( + PEER_ASN, + PEER_ID, + 90, + vec![four_octet(), AddressFamily::Ipv4Unicast.capability()], + ) + .await?; + wait_for_session(&server).await?; + + peer.send(&Message::Update(dual_stack_update())).await?; + + let routes = tokio::time::timeout(TIMEOUT, async { + loop { + let routes = server.adj_rib_in.snapshot().await; + if !routes.is_empty() { + return routes; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await?; + // One UPDATE is applied as a whole, so the IPv6 route is not on its way. + assert_eq!(routes.len(), 1); + assert_eq!(routes[0].prefix, prefix("192.0.2.0/24")); + assert_eq!(server.registry.list().len(), 1, "the session survives"); + Ok(()) +} + +/// A route of a family the peer did not offer is not sent to it. +#[tokio::test] +async fn test_routes_of_a_family_not_negotiated_are_not_sent() -> Result<()> { + let server = TestServer::start_with(LOCAL_ASN, |c| { + c.originate = vec![LocalRoute::new( + prefix("10.100.0.0/16"), + LocalRouteAttrs::default(), + )]; + }) + .await?; + + // This peer wants IPv6 only; the server has an IPv4 route. + let mut v6_only = TestPeer::connect(server.addr).await?; + v6_only + .handshake_with( + PEER_ASN, + PEER_ID, + 90, + vec![four_octet(), AddressFamily::Ipv6Unicast.capability()], + ) + .await?; + let session = wait_for_session(&server).await?; + assert_eq!(session.families, only(AddressFamily::Ipv6Unicast)); + + // A second peer that takes IPv4 is sent the route. The advertiser + // handles sessions in the order they came up, so by the time this one + // has its route the first has been dealt with. + let mut dual = TestPeer::connect(server.addr).await?; + dual.handshake(PEER_ASN, Ipv4Addr::new(10, 0, 0, 2), 90) + .await?; + let update = tokio::time::timeout(TIMEOUT, dual.recv_update()).await??; + assert_eq!(update.announced_routes, vec![prefix("10.100.0.0/16")]); + + let nothing = tokio::time::timeout(Duration::from_millis(200), v6_only.recv_update()).await; + assert!(nothing.is_err(), "got {nothing:?}"); + Ok(()) +} + +/// A configured peer is offered only the families configured for it, and a +/// peer that shares none of them is refused with the capability we needed. +#[tokio::test] +async fn test_no_family_in_common_refuses_the_session() -> Result<()> { + let server = TestServer::start_with(LOCAL_ASN, |c| { + // Inbound connections from this address inherit the peer's settings. + c.peers = vec![PeerConfig { + name: "v6-only".into(), + endpoint: SocketAddr::new(Ipv4Addr::LOCALHOST.into(), 1), + remote_asn: PEER_ASN, + hold_time_secs: 90, + local_address: None, + families: only(AddressFamily::Ipv6Unicast), + }]; + // Keep the dialer out of the way. + c.connect_retry = Duration::from_secs(3600); + }) + .await?; + + let mut peer = TestPeer::connect(server.addr).await?; + peer.send_open_with( + PEER_ASN, + PEER_ID, + 90, + vec![four_octet(), AddressFamily::Ipv4Unicast.capability()], + ) + .await?; + let open = peer.recv_open().await?; + let offered: Vec<_> = open + .capabilities() + .filter(|c| matches!(c, Capability::MultiProtocol { .. })) + .collect(); + assert_eq!(offered, [&AddressFamily::Ipv6Unicast.capability()]); + + match tokio::time::timeout(TIMEOUT, peer.recv()).await?? { + Message::Notification(n) => { + assert_eq!( + n.error, + BgpError::OpenMsg(OpenMsgSubcode::UnsupportedCapability) + ); + // The Multiprotocol capability for IPv6 unicast. + assert_eq!(n.data, [1, 4, 0, 2, 0, 1]); + } + other => bail!("expected NOTIFICATION, got {other:?}"), + } + assert!(server.registry.list().is_empty()); + Ok(()) +} + +/// Real routers offer families we have never heard of. That must not stop +/// the families we do share from working. +#[tokio::test] +async fn test_a_peer_offering_an_unknown_family_is_still_accepted() -> Result<()> { + let server = TestServer::start(LOCAL_ASN).await?; + let mut peer = TestPeer::connect(server.addr).await?; + peer.handshake_with( + PEER_ASN, + PEER_ID, + 90, + vec![ + four_octet(), + // Multiprotocol for L2VPN EVPN (AFI 25, SAFI 70). + Capability::UnknownCapability { + type_code: 1, + length: 4, + value: vec![0, 25, 0, 70], + }, + AddressFamily::Ipv4Unicast.capability(), + ], + ) + .await?; + + let session = wait_for_session(&server).await?; + assert_eq!(session.families, only(AddressFamily::Ipv4Unicast)); + assert!( + session.four_octet_asn, + "capabilities after it are still read" + ); + Ok(()) +} + +/// A peer that offers no capabilities at all is a plain RFC 4271 speaker: +/// IPv4 unicast, two-octet AS numbers. Our four-octet AS number reaches it +/// as AS_TRANS in AS_PATH and in full in AS4_PATH (RFC 6793 §4.2.2). +#[tokio::test] +async fn test_a_peer_without_capabilities_gets_two_octet_as_paths() -> Result<()> { + const WIDE_ASN: u32 = 4_200_000_001; + let server = TestServer::start_with(WIDE_ASN, |c| { + c.originate = vec![LocalRoute::new( + prefix("10.100.0.0/16"), + LocalRouteAttrs::default(), + )]; + }) + .await?; + + let mut peer = TestPeer::connect(server.addr).await?; + peer.handshake_with(PEER_ASN, PEER_ID, 90, vec![]).await?; + let session = wait_for_session(&server).await?; + assert!(!session.four_octet_asn); + assert_eq!(session.families, only(AddressFamily::Ipv4Unicast)); + + // The test peer now decodes AS_PATH two octets wide, as such a speaker + // would; an UPDATE encoded four octets wide would not parse. + let update = tokio::time::timeout(TIMEOUT, peer.recv_update()).await??; + assert_eq!(update.announced_routes, vec![prefix("10.100.0.0/16")]); + assert!( + update + .path_attributes + .contains(&AsPathAttribute::from_asns(vec![23456])), + "{update:?}" + ); + let as4_path = update.path_attributes.iter().find_map(|a| match a { + PathAttribute::As4Path(path) => Some(path.segments[0].path.clone()), + _ => None, + }); + assert_eq!(as4_path, Some(vec![WIDE_ASN])); + + // And its own two-octet paths are read correctly. + peer.send(&Message::Update(UpdateMessage { + withdrawn_routes: vec![], + path_attributes: vec![ + PathAttribute::Origin(OriginPathAttribute::IGP), + AsPathAttribute::from_asns(vec![PEER_ASN, 64512]), + PathAttribute::NextHop(NextHopPathAttribute(Ipv4Addr::new(192, 0, 2, 1))), + ], + announced_routes: vec![prefix("192.0.2.0/24")], + })) + .await?; + let routes = tokio::time::timeout(TIMEOUT, async { + loop { + let routes = server.adj_rib_in.snapshot().await; + if !routes.is_empty() { + return routes; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await?; + assert!( + routes[0] + .entry + .path_attributes + .contains(&AsPathAttribute::from_asns(vec![PEER_ASN, 64512])) + ); + Ok(()) +} diff --git a/crates/packet/src/message.rs b/crates/packet/src/message.rs index c646f68..b2950b5 100644 --- a/crates/packet/src/message.rs +++ b/crates/packet/src/message.rs @@ -120,6 +120,8 @@ impl Message { ProtocolErrorValues::UnknownOpenOption(_) => { OpenMsgSubcode::UnsupportedOptionalParam } + // Handled inside the capability parser. + ProtocolErrorValues::UnknownAddressFamily => OpenMsgSubcode::Unspecific, }, _ => OpenMsgSubcode::Unspecific, }; @@ -401,6 +403,9 @@ pub enum PathAttribute { MpReachNlri(MpReachNlriPathAttribute) = 14, MpUnreachNlri(MpUnreachNlriPathAttribute) = 15, ExtendedCommunities(ExtendedCommunitiesPathAttribute) = 16, + /// The AS path with four-octet AS numbers, for a route that crosses + /// speakers that only handle two-octet ones (RFC 6793 §3). + As4Path(AsPathAttribute) = 17, LargeCommunities(LargeCommunitiesPathAttribute) = 32, UnknownPathAttribute { flags: PathAttributeFlags, @@ -423,6 +428,7 @@ impl From for PathAttributeDiscriminants { v if v == Self::MpReachNlri as u8 => Self::MpReachNlri, v if v == Self::MpUnreachNlri as u8 => Self::MpUnreachNlri, v if v == Self::ExtendedCommunities as u8 => Self::ExtendedCommunities, + v if v == Self::As4Path as u8 => Self::As4Path, v if v == Self::LargeCommunities as u8 => Self::LargeCommunities, _ => Self::UnknownPathAttribute, } @@ -445,6 +451,7 @@ impl PathAttribute { PathAttribute::ExtendedCommunities(_) => { PathAttributeDiscriminants::ExtendedCommunities as u8 } + PathAttribute::As4Path(_) => PathAttributeDiscriminants::As4Path as u8, PathAttribute::LargeCommunities(_) => { PathAttributeDiscriminants::LargeCommunities as u8 } @@ -465,6 +472,7 @@ impl PathAttribute { PathAttribute::MpReachNlri(_) => true, PathAttribute::MpUnreachNlri(_) => true, PathAttribute::ExtendedCommunities(_) => true, + PathAttribute::As4Path(_) => true, PathAttribute::LargeCommunities(_) => true, PathAttribute::UnknownPathAttribute { flags, .. } => flags.optional(), } @@ -483,6 +491,7 @@ impl PathAttribute { PathAttribute::MpReachNlri(_) => false, PathAttribute::MpUnreachNlri(_) => false, PathAttribute::ExtendedCommunities(_) => true, + PathAttribute::As4Path(_) => true, PathAttribute::LargeCommunities(_) => true, PathAttribute::UnknownPathAttribute { flags, .. } => flags.transitive(), } @@ -563,6 +572,9 @@ impl PathAttribute { PathAttributeDiscriminants::ExtendedCommunities => PathAttribute::ExtendedCommunities( ExtendedCommunitiesPathAttribute::from_wire(ctx, pa_buf)?.1, ), + PathAttributeDiscriminants::As4Path => { + PathAttribute::As4Path(AsPathAttribute::from_wire_with_width(true, pa_buf)?.1) + } PathAttributeDiscriminants::LargeCommunities => PathAttribute::LargeCommunities( LargeCommunitiesPathAttribute::from_wire(ctx, pa_buf)?.1, ), @@ -637,6 +649,16 @@ impl PathAttribute { PathAttribute::LargeCommunities(large_communities_path_attribute) => { write_path_attribute!(out, large_communities_path_attribute); } + PathAttribute::As4Path(as4_path) => { + // Always four octets wide, whatever the session's AS_PATH is. + let wire_len = as4_path.wire_len_with_width(true)?; + let attr_flags = + PathAttributeFlags::new(true, true, /*partial=*/ false, wire_len > 255); + out.put_u8(*attr_flags); + out.put_u8(self.discriminant()); + write_wire_len!(out, wire_len); + as4_path.to_wire_with_width(true, out)?; + } PathAttribute::UnknownPathAttribute { flags, type_code, @@ -671,6 +693,7 @@ impl PathAttribute { PathAttribute::MpReachNlri(a) => a.wire_len(ctx)?, PathAttribute::MpUnreachNlri(a) => a.wire_len(ctx)?, PathAttribute::ExtendedCommunities(a) => a.wire_len(ctx)?, + PathAttribute::As4Path(a) => a.wire_len_with_width(true)?, PathAttribute::LargeCommunities(a) => a.wire_len(ctx)?, PathAttribute::UnknownPathAttribute { flags, payload, .. } => { // An unknown attribute preserves its original extended_length @@ -775,51 +798,57 @@ impl AsPathAttribute { }) } + /// Parses an AS_PATH. How wide its AS numbers are depends on what the + /// session negotiated (RFC 6793), so the context must say. pub fn from_wire<'a>( ctx: &ParserContext, buf: &'a [u8], ) -> IResult<&'a [u8], Self, BgpParserError<&'a [u8]>> { - let parse_segment = |ctx: &ParserContext, - buf: &'a [u8]| - -> IResult<&'a [u8], AsPathSegment, BgpParserError<&'a [u8]>> { + let Some(four_octet) = ctx.four_octet_asn else { + return Err(nom::Err::Failure(BgpParserError::Malformed( + "Context must set four_octet_asn before being used".into(), + ))); + }; + Self::from_wire_with_width(four_octet, buf) + } + + fn from_wire_with_width( + four_octet: bool, + buf: &[u8], + ) -> IResult<&[u8], Self, BgpParserError<&[u8]>> { + let parse_segment = |buf| -> IResult<&[u8], AsPathSegment, BgpParserError<&[u8]>> { let (buf, typ) = be_u8(buf)?; let (buf, len) = be_u8(buf)?; - let (buf, asns): (_, Vec) = match ctx.four_octet_asn { - Some(true) => { - nom::multi::many_m_n(len as usize, len as usize, be_u32).parse(buf)? - } - Some(false) => nom::multi::many_m_n(len as usize, len as usize, be_u16) + let count = usize::from(len); + let (buf, path): (_, Vec) = if four_octet { + nom::multi::many_m_n(count, count, be_u32).parse(buf)? + } else { + nom::multi::many_m_n(count, count, be_u16) .parse(buf) - .map(|(buf, asns)| (buf, asns.iter().map(|asn| *asn as u32).collect()))?, - None => { - return Err(nom::Err::Failure(BgpParserError::Malformed( - "Context must set four_octet_asn before being used".into(), - ))); - } + .map(|(buf, asns)| (buf, asns.into_iter().map(u32::from).collect()))? }; - Ok(( buf, AsPathSegment { ordered: typ == 2, - path: asns, + path, }, )) }; - let (buf, segments) = - nom::multi::many0(|buf: &'a [u8]| parse_segment(ctx, buf)).parse(buf)?; - + let (buf, segments) = nom::multi::many0(parse_segment).parse(buf)?; Ok((buf, Self { segments })) } + /// Writes the path with AS numbers as wide as the session negotiated. + /// On a two-octet session an AS number that does not fit is written as + /// AS_TRANS (RFC 6793 §4.2.2); [`AsPathAttribute::as4_path`] is what + /// carries the real numbers alongside. pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { - if ctx.four_octet_asn.is_none_or(|val| !val) { - return Err(EncodeError::Unencodable( - "AS_PATH can only be sent to peers with four-octet AS numbers", - )); - } + self.to_wire_with_width(width(ctx)?, out) + } + fn to_wire_with_width(&self, four_octet: bool, out: &mut BytesMut) -> Result<(), EncodeError> { for segment in &self.segments { // Segment type. out.put_u8(if segment.ordered { 2 } else { 1 }); @@ -835,43 +864,52 @@ impl AsPathAttribute { max: u8::MAX.into(), })?, ); - // AS numbers. - match ctx.four_octet_asn.ok_or(EncodeError::Unencodable( - "the AS number width has not been negotiated", - ))? { - true => { - for asn in &segment.path { - out.put_u32(*asn); - } - } - false => { - for asn in &segment.path { - out.put_u16(u16::try_from(*asn).map_err(|_| { - EncodeError::Unencodable("AS number does not fit in two octets") - })?); - } + for asn in &segment.path { + if four_octet { + out.put_u32(*asn); + } else { + out.put_u16(u16::try_from(*asn).unwrap_or(AS_TRANS)); } } } - Ok(()) } pub fn wire_len(&self, ctx: &ParserContext) -> Result { - let mut counter = 0; - for segment in &self.segments { - counter += match ctx.four_octet_asn { - Some(true) => 2 + (4 * segment.path.len()), - Some(false) => 2 + (2 * segment.path.len()), - None => { - return Err(EncodeError::Unencodable( - "the AS number width has not been negotiated", - )); - } - }; - } - Ok(counter as u16) + self.wire_len_with_width(width(ctx)?) } + + fn wire_len_with_width(&self, four_octet: bool) -> Result { + let asn_len = if four_octet { 4 } else { 2 }; + let total: usize = self + .segments + .iter() + .map(|segment| 2 + asn_len * segment.path.len()) + .sum(); + u16::try_from(total).map_err(|_| EncodeError::TooLong { + what: "AS path", + len: total, + max: u16::MAX.into(), + }) + } + + /// The AS4_PATH to send with this path on a two-octet session, if one is + /// needed: it is when some AS number does not fit in two octets and so + /// goes out as AS_TRANS in the AS_PATH itself (RFC 6793 §4.2.2). + pub fn as4_path(&self) -> Option { + self.segments + .iter() + .flat_map(|segment| &segment.path) + .any(|asn| u16::try_from(*asn).is_err()) + .then(|| PathAttribute::As4Path(self.clone())) + } +} + +/// How wide AS numbers are on the session `ctx` describes. +fn width(ctx: &ParserContext) -> Result { + ctx.four_octet_asn.ok_or(EncodeError::Unencodable( + "the AS number width has not been negotiated", + )) } #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] @@ -1987,6 +2025,73 @@ mod tests { ); // Rejects trailing garbage after the NLRI section (a partial prefix). + /// AS_PATH is as wide as the session negotiated; AS4_PATH is always + /// four octets wide (RFC 6793). + #[test] + fn as_paths_are_encoded_as_wide_as_the_session_allows() { + let path = AsPathAttribute { + segments: vec![AsPathSegment { + ordered: true, + path: vec![65000, 4_200_000_001], + }], + }; + let two_octet = ParserContext { + four_octet_asn: Some(false), + address_family: None, + }; + let four_octet = ParserContext { + four_octet_asn: Some(true), + address_family: None, + }; + let wire = |attribute: &PathAttribute, ctx: &ParserContext| { + let mut out = BytesMut::new(); + attribute.to_wire(ctx, &mut out).unwrap(); + assert_eq!(out.len(), usize::from(attribute.wire_len(ctx).unwrap())); + out.to_vec() + }; + + // Two octets: the number that does not fit goes out as AS_TRANS. + let as_path = PathAttribute::AsPath(path.clone()); + #[rustfmt::skip] + assert_eq!( + wire(&as_path, &two_octet), + [0x40, 0x02, 0x06, 0x02, 0x02, 0xfd, 0xe8, 0x5b, 0xa0] + ); + #[rustfmt::skip] + assert_eq!( + wire(&as_path, &four_octet), + [0x40, 0x02, 0x0a, 0x02, 0x02, 0x00, 0x00, 0xfd, 0xe8, 0xfa, 0x56, 0xea, 0x01] + ); + + // AS4_PATH: optional transitive, type 17, the same on any session. + let as4_path = path.as4_path().expect("one AS number needs four octets"); + #[rustfmt::skip] + let expected = [0xc0, 0x11, 0x0a, 0x02, 0x02, 0x00, 0x00, 0xfd, 0xe8, 0xfa, 0x56, 0xea, 0x01]; + for ctx in [&two_octet, &four_octet] { + let bytes = wire(&as4_path, ctx); + assert_eq!(bytes, expected); + let (rest, parsed) = PathAttribute::from_wire(ctx, &bytes).unwrap(); + assert!(rest.is_empty()); + assert_eq!(parsed, as4_path); + } + + // A path of numbers that all fit needs no AS4_PATH. + let narrow = AsPathAttribute { + segments: vec![AsPathSegment { + ordered: true, + path: vec![65000, 65001], + }], + }; + assert_eq!(narrow.as4_path(), None); + + // Without a negotiated width there is nothing right to write. + assert!( + as_path + .to_wire(&ParserContext::default(), &mut BytesMut::new()) + .is_err() + ); + } + #[test] fn test_update_rejects_trailing_garbage() { let ctx = default_v6_context(); diff --git a/crates/packet/src/open.rs b/crates/packet/src/open.rs index 7246940..e9656bf 100644 --- a/crates/packet/src/open.rs +++ b/crates/packet/src/open.rs @@ -97,6 +97,14 @@ impl OpenMessage { )) } + /// Every capability in the message, across all the optional parameters + /// that carry them. + pub fn capabilities(&self) -> impl Iterator { + self.options.iter().flat_map(|option| match option { + OpenOption::Capabilities(capabilities) => capabilities.iter(), + }) + } + pub fn to_wire(&self, ctx: &ParserContext, out: &mut BytesMut) -> Result<(), EncodeError> { out.put_u8(self.version); out.put_u16(self.asn); @@ -214,6 +222,25 @@ impl OpenOption { } } +/// An AFI in a capability, or the marker that makes the capability unknown. +fn known_afi<'a>(afi: u16) -> Result>> { + AddressFamilyId::try_from(afi).map_err(|_| unknown_family()) +} + +/// A SAFI in a capability, or the marker that makes the capability unknown. +fn known_safi<'a, T>(safi: T) -> Result>> +where + SubsequentAfi: TryFrom, +{ + SubsequentAfi::try_from(safi).map_err(|_| unknown_family()) +} + +fn unknown_family<'a>() -> nom::Err> { + Failure(BgpParserError::ProtocolError( + ProtocolErrorValues::UnknownAddressFamily, + )) +} + /// One address family in a Multiple Labels capability (RFC 8277 §2.1). #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] pub struct MultiLabelEntry { @@ -324,6 +351,11 @@ impl Capability { } } /// `from_wire` parses a Capability message. + /// + /// A capability we know, about an address family we do not, is kept as + /// an [`Capability::UnknownCapability`]: a peer is free to advertise + /// families we have never heard of, and RFC 5492 §3 has us ignore what + /// we do not support rather than refuse the OPEN. pub fn from_wire<'a>( _ctx: &ParserContext, buf: &'a [u8], @@ -331,10 +363,28 @@ impl Capability { let (buf, typ) = be_u8(buf)?; let (buf, len) = be_u8(buf)?; // `cap_buf` holds exactly this capability's value; `buf` is what - // follows it. Every arm must return `buf`, or the caller's list - // parser sees an empty remainder and silently drops all the - // capabilities after this one. + // follows it and is what the caller's list parser continues with. let (buf, cap_buf) = nom::bytes::take(len).parse(buf)?; + let capability = match Self::parse_value(typ, cap_buf) { + Ok(capability) => capability, + Err(Failure(BgpParserError::ProtocolError( + ProtocolErrorValues::UnknownAddressFamily, + ))) => Self::UnknownCapability { + type_code: typ, + length: len, + value: cap_buf.to_vec(), + }, + Err(e) => return Err(e), + }; + Ok((buf, capability)) + } + + /// Parses the value of a capability of type `typ`. `cap_buf` is exactly + /// the value. + fn parse_value<'a>( + typ: u8, + cap_buf: &'a [u8], + ) -> Result>> { macro_rules! require_empty { ($buf:expr, $cap_type:expr) => { if !$buf.is_empty() { @@ -352,20 +402,20 @@ impl Capability { let (cap_buf, _) = be_u8(cap_buf)?; let (cap_buf, safi) = be_u8(cap_buf)?; require_empty!(cap_buf, "MultiProtocol"); - let afi = AddressFamilyId::try_from(afi).map_err(|e| fail!("{}", e))?; - let safi = SubsequentAfi::try_from(safi).map_err(|e| fail!("{}", e))?; - Ok((buf, Self::MultiProtocol { afi, safi })) + let afi = known_afi(afi)?; + let safi = known_safi(safi)?; + Ok(Self::MultiProtocol { afi, safi }) } v if v == CapabilityDiscriminants::RouteRefresh as u8 => { require_empty!(cap_buf, "RouteRefresh"); - Ok((buf, Self::RouteRefresh)) + Ok(Self::RouteRefresh) } v if v == CapabilityDiscriminants::OutboundRouteFilter as u8 => { let (cap_buf, afi) = be_u16(cap_buf)?; let (cap_buf, _) = be_u8(cap_buf)?; // Move past reverved byte. let (cap_buf, safi) = be_u8(cap_buf)?; - let afi = AddressFamilyId::try_from(afi).map_err(|e| fail!("{}", e))?; - let safi = SubsequentAfi::try_from(safi).map_err(|e| fail!("{}", e))?; + let afi = known_afi(afi)?; + let safi = known_safi(safi)?; let (cap_buf, _num_orfs) = be_u8(cap_buf)?; @@ -392,7 +442,7 @@ impl Capability { let (cap_buf, orfs) = nom::multi::many0(parse_orf).parse(cap_buf)?; require_empty!(cap_buf, "OutboundRouteFilter"); - Ok((buf, Self::OutboundRouteFilter { afi, safi, orfs })) + Ok(Self::OutboundRouteFilter { afi, safi, orfs }) } v if v == CapabilityDiscriminants::ExtendedNextHop as u8 => { // parse_enh parses a single extended nexthop entry. @@ -404,32 +454,29 @@ impl Capability { let (buf, afi) = be_u16(buf)?; let (buf, safi) = be_u16(buf)?; let (buf, nh_afi) = be_u16(buf)?; - let afi = AddressFamilyId::try_from(afi).map_err(|e| fail!("{}", e))?; - let safi = SubsequentAfi::try_from(safi).map_err(|e| fail!("{}", e))?; - let nh_afi = AddressFamilyId::try_from(nh_afi).map_err(|e| fail!("{}", e))?; + let afi = known_afi(afi)?; + let safi = known_safi(safi)?; + let nh_afi = known_afi(nh_afi)?; Ok((buf, ExtendedNextHop { afi, safi, nh_afi })) }; let (cap_buf, entries) = nom::multi::many1(parse_enh).parse(cap_buf)?; require_empty!(cap_buf, "ExtendedNextHop"); - Ok((buf, Self::ExtendedNextHop { entries })) + Ok(Self::ExtendedNextHop { entries }) } v if v == CapabilityDiscriminants::ExtendedMessage as u8 => { require_empty!(cap_buf, "ExtendedMessage"); - Ok((buf, Self::ExtendedMessage)) + Ok(Self::ExtendedMessage) } v if v == CapabilityDiscriminants::BgpSec as u8 => { let (cap_buf, header) = be_u8(cap_buf)?; let (cap_buf, afi) = be_u16(cap_buf)?; - let afi = AddressFamilyId::try_from(afi).map_err(|e| fail!("{}", e))?; + let afi = known_afi(afi)?; require_empty!(cap_buf, "BgpSec"); - Ok(( - buf, - Self::BgpSec { - header: BgpSecHeader(header), - afi, - }, - )) + Ok(Self::BgpSec { + header: BgpSecHeader(header), + afi, + }) } v if v == CapabilityDiscriminants::MultiLabelCompat as u8 => { // The AFI and SAFI are kept as numbers: this capability is @@ -446,13 +493,13 @@ impl Capability { }; let (cap_buf, entries) = nom::multi::many1(parse_entry).parse(cap_buf)?; require_empty!(cap_buf, "MultiLabelCompat"); - Ok((buf, Self::MultiLabelCompat { entries })) + Ok(Self::MultiLabelCompat { entries }) } v if v == CapabilityDiscriminants::BgpRole as u8 => { let (cap_buf, role) = be_u8(cap_buf)?; let role = BgpRole::from(role); require_empty!(cap_buf, "BgpRole"); - Ok((buf, Self::BgpRole { role })) + Ok(Self::BgpRole { role }) } v if v == CapabilityDiscriminants::GracefulRestart as u8 => { // flags_restart_time is: (4 bits: restart flags, 12 bits: restart time (s)). @@ -467,8 +514,8 @@ impl Capability { let (buf, afi) = be_u16(buf)?; let (buf, safi) = be_u8(buf)?; let (buf, flags) = be_u8(buf)?; - let afi = AddressFamilyId::try_from(afi).map_err(|e| fail!("{}", e))?; - let safi = SubsequentAfi::try_from(safi).map_err(|e| fail!("{}", e))?; + let afi = known_afi(afi)?; + let safi = known_safi(safi)?; Ok(( buf, GracefulRestartEntry { @@ -480,40 +527,34 @@ impl Capability { }; let (cap_buf, entries) = nom::multi::many0(parse_entry).parse(cap_buf)?; require_empty!(cap_buf, "GracefulRestart"); - Ok(( - buf, - Self::GracefulRestart { - restart_state, - restart_time, - entries, - }, - )) + Ok(Self::GracefulRestart { + restart_state, + restart_time, + entries, + }) } v if v == CapabilityDiscriminants::FourByteAsn as u8 => { let (cap_buf, asn) = be_u32(cap_buf)?; require_empty!(cap_buf, "FourByteAsn"); - Ok((buf, Self::FourByteAsn { asn })) + Ok(Self::FourByteAsn { asn }) } v if v == CapabilityDiscriminants::AddPath as u8 => { let (cap_buf, afi) = be_u16(cap_buf)?; let (cap_buf, safi) = be_u8(cap_buf)?; let (cap_buf, send_recv) = be_u8(cap_buf)?; - let afi = AddressFamilyId::try_from(afi).map_err(|e| fail!("{}", e))?; - let safi = SubsequentAfi::try_from(safi).map_err(|e| fail!("{}", e))?; + let afi = known_afi(afi)?; + let safi = known_safi(safi)?; let send_recv = AddPathSendRecv::try_from(send_recv).map_err(|e| fail!("{}", e))?; require_empty!(cap_buf, "AddPath"); - Ok(( - buf, - Self::AddPath { - afi, - safi, - send_recv, - }, - )) + Ok(Self::AddPath { + afi, + safi, + send_recv, + }) } v if v == CapabilityDiscriminants::EnhancedRouteRefresh as u8 => { require_empty!(cap_buf, "EnhancedRouteRefresh"); - Ok((buf, Self::EnhancedRouteRefresh)) + Ok(Self::EnhancedRouteRefresh) } v if v == CapabilityDiscriminants::LongLivedGracefulRestart as u8 => { let parse_llgr = |buf: &'a [u8]| -> IResult< @@ -525,8 +566,8 @@ impl Capability { let (buf, safi) = be_u8(buf)?; let (buf, flags) = be_u8(buf)?; let (buf, stale_time) = be_u24(buf)?; - let afi = AddressFamilyId::try_from(afi).map_err(|e| fail!("{}", e))?; - let safi = SubsequentAfi::try_from(safi).map_err(|e| fail!("{}", e))?; + let afi = known_afi(afi)?; + let safi = known_safi(safi)?; Ok(( buf, LongLivedGracefulRestart { @@ -539,19 +580,16 @@ impl Capability { }; let (cap_buf, entries) = nom::multi::many0(parse_llgr).parse(cap_buf)?; require_empty!(cap_buf, "LongLivedGracefulRestart"); - Ok((buf, Self::LongLivedGracefulRestart(entries))) + Ok(Self::LongLivedGracefulRestart(entries)) } type_code => { // Handling an unknown capability, so just parse it out and store the value // as bytes. - Ok(( - buf, - Self::UnknownCapability { - type_code, - length: cap_buf.len() as u8, - value: cap_buf.to_vec(), - }, - )) + Ok(Self::UnknownCapability { + type_code, + length: cap_buf.len() as u8, + value: cap_buf.to_vec(), + }) } } } @@ -1070,6 +1108,43 @@ mod tests { assert!(Capability::from_wire(&ctx, &[0x08, 0x03, 0x00, 0x01, 0x04]).is_err()); } + /// A peer may offer families we have no name for (here L2VPN EVPN, + /// AFI 25 SAFI 70). That used to fail the whole OPEN. It is kept as an + /// unknown capability, byte for byte, and what follows is still read. + #[test] + fn a_capability_for_an_unknown_address_family_is_kept_as_unknown() { + let ctx = ParserContext::default(); + let samples: &[(&str, &[u8])] = &[ + ("multiprotocol", &[0x01, 0x04, 0x00, 0x19, 0x00, 0x46]), + ("add-path", &[0x45, 0x04, 0x00, 0x19, 0x46, 0x03]), + ( + "graceful restart", + &[0x40, 0x06, 0x00, 0x78, 0x00, 0x19, 0x46, 0x80], + ), + ]; + for (name, wire) in samples { + let mut input = wire.to_vec(); + input.extend_from_slice(&[0x02, 0x00]); + let (rest, cap) = Capability::from_wire(&ctx, &input).unwrap(); + assert_eq!(rest, [0x02, 0x00], "{name}"); + assert_eq!( + cap, + Capability::UnknownCapability { + type_code: wire[0], + length: wire[1], + value: wire[2..].to_vec(), + }, + "{name}" + ); + let mut out = BytesMut::new(); + cap.to_wire(&ctx, &mut out).unwrap(); + assert_eq!(&out[..], *wire, "{name}"); + } + + // A capability that is simply malformed is still an error. + assert!(Capability::from_wire(&ctx, &[0x01, 0x03, 0x00, 0x01, 0x00]).is_err()); + } + #[test] fn capability_parsers_leave_the_following_bytes() { let ctx = ParserContext::default(); diff --git a/crates/primitives/src/parser.rs b/crates/primitives/src/parser.rs index 12d614a..e049030 100644 --- a/crates/primitives/src/parser.rs +++ b/crates/primitives/src/parser.rs @@ -57,4 +57,8 @@ pub enum ProtocolErrorValues { UnsupportedVersion(u8), /// `UnknownOpenOption` is returned with the open option type code that is unknown. UnknownOpenOption(u8), + /// A capability names an address family this implementation has no name + /// for. The capability parser turns this into an unknown capability; it + /// does not reach callers. + UnknownAddressFamily, } diff --git a/crates/server/proto/config.proto b/crates/server/proto/config.proto index c015a9e..af33453 100644 --- a/crates/server/proto/config.proto +++ b/crates/server/proto/config.proto @@ -91,4 +91,17 @@ message Peer { // same machine. Must be the same address family as the endpoint. Unset // lets the operating system choose. string local_address = 5; + + // The address families to exchange with this peer. Each is offered to the + // peer in our OPEN, and is in use on the session only if the peer offers + // it too (RFC 4760). Defaults to all of them when empty. A session with + // no family in common is refused. + repeated AddressFamily address_families = 6; +} + +// An address family routes can be exchanged for. +enum AddressFamily { + ADDRESS_FAMILY_UNSPECIFIED = 0; + IPV4_UNICAST = 1; + IPV6_UNICAST = 2; } diff --git a/crates/server/src/advertise.rs b/crates/server/src/advertise.rs index bbd1bda..41926a7 100644 --- a/crates/server/src/advertise.rs +++ b/crates/server/src/advertise.rs @@ -10,8 +10,8 @@ use std::net::IpAddr; use bgp_packet::message::{ - AsPathAttribute, CommunitiesPathAttribute, LargeCommunitiesPathAttribute, Message, - MpReachNlriPathAttribute, MpUnreachNlriPathAttribute, MultiExitDiscPathAttribute, + AsPathAttribute, AsPathSegment, CommunitiesPathAttribute, LargeCommunitiesPathAttribute, + Message, MpReachNlriPathAttribute, MpUnreachNlriPathAttribute, MultiExitDiscPathAttribute, NextHopPathAttribute, NlriNextHop, PathAttribute, UpdateMessage, }; use bgp_primitives::constants::{AddressFamilyId, SubsequentAfi}; @@ -90,7 +90,20 @@ async fn resync_all(local_rib: &LocalRib, registry: &SessionRegistry, local_asn: } async fn send_route(session: &SessionHandle, route: &LocalRoute, local_asn: u32) { - match build_announce(route, local_asn, session.local_addr.ip()) { + if !session.families.carries(&route.prefix) { + debug!( + peer = %session.peer, + prefix = %route.prefix, + "Skipping route: its address family was not negotiated" + ); + return; + } + match build_announce( + route, + local_asn, + session.local_addr.ip(), + session.four_octet_asn, + ) { Some(update) => { debug!(peer = %session.peer, prefix = %route.prefix, "Announcing local route"); if let Err(e) = session.send(Message::Update(update)).await { @@ -106,6 +119,10 @@ async fn send_route(session: &SessionHandle, route: &LocalRoute, local_asn: u32) } async fn send_withdraw(session: &SessionHandle, prefix: &IpPrefix) { + if !session.families.carries(prefix) { + // It was never announced on this session either. + return; + } debug!(peer = %session.peer, %prefix, "Withdrawing local route"); if let Err(e) = session.send(Message::Update(build_withdraw(prefix))).await { warn!(peer = %session.peer, err = %e, "Failed to queue withdraw"); @@ -117,18 +134,36 @@ async fn send_withdraw(session: &SessionHandle, prefix: &IpPrefix) { /// Returns `None` when the prefix family does not match the next-hop family: /// carrying IPv4 NLRI over an IPv6 next hop (RFC 8950) or vice versa is not /// supported yet. +/// +/// `four_octet_asn` is whether the session the UPDATE is for negotiated +/// four-octet AS numbers. If it did not, and the local AS number needs four +/// octets, the path is also sent as AS4_PATH so that it survives the +/// two-octet speaker (RFC 6793 §4.2.2). pub fn build_announce( route: &LocalRoute, local_asn: u32, next_hop: IpAddr, + four_octet_asn: bool, ) -> Option { let attrs = &route.attrs; + let as_path = AsPathAttribute { + segments: vec![AsPathSegment { + ordered: true, + path: vec![local_asn; 1 + usize::from(attrs.prepend)], + }], + }; + let as4_path = if four_octet_asn { + None + } else { + as_path.as4_path() + }; + // Path attributes in ascending type-code order, as most implementations // expect them. let mut path_attributes = vec![ PathAttribute::Origin(attrs.origin), - AsPathAttribute::from_asns(vec![local_asn; 1 + attrs.prepend as usize]), + PathAttribute::AsPath(as_path), ]; let mut announced_routes = vec![]; @@ -159,6 +194,7 @@ pub fn build_announce( prefixes: vec![route.prefix.clone()], })); } + path_attributes.extend(as4_path); if !attrs.large_communities.is_empty() { path_attributes.push(PathAttribute::LargeCommunities( LargeCommunitiesPathAttribute { @@ -226,7 +262,7 @@ mod tests { }, ); let update = - build_announce(&route, 65000, IpAddr::V4(Ipv4Addr::new(192, 0, 2, 1))).unwrap(); + build_announce(&route, 65000, IpAddr::V4(Ipv4Addr::new(192, 0, 2, 1)), true).unwrap(); assert_eq!(update.announced_routes, vec![prefix("10.100.1.0/24")]); assert!(update.withdrawn_routes.is_empty()); @@ -270,7 +306,7 @@ mod tests { fn ipv6_announce_uses_mp_reach() { let route = LocalRoute::new(prefix("2001:db8:1::/48"), LocalRouteAttrs::default()); let nh = Ipv6Addr::new(0x2001, 0xdb8, 0, 0, 0, 0, 0, 1); - let update = build_announce(&route, 65000, IpAddr::V6(nh)).unwrap(); + let update = build_announce(&route, 65000, IpAddr::V6(nh), true).unwrap(); assert!(update.announced_routes.is_empty()); assert!( @@ -286,12 +322,57 @@ mod tests { ))); } + /// A two-octet session cannot carry a four-octet AS number in AS_PATH: + /// it goes out as AS_TRANS there, and in full in AS4_PATH. + #[test] + fn a_wide_as_number_reaches_a_two_octet_peer_in_as4_path() { + let route = LocalRoute::new( + prefix("10.100.1.0/24"), + LocalRouteAttrs { + prepend: 1, + ..LocalRouteAttrs::default() + }, + ); + let next_hop = IpAddr::V4(Ipv4Addr::new(192, 0, 2, 1)); + let path = |update: &UpdateMessage, as4: bool| { + update.path_attributes.iter().find_map(|a| match a { + PathAttribute::As4Path(p) if as4 => Some(p.segments[0].path.clone()), + PathAttribute::AsPath(p) if !as4 => Some(p.segments[0].path.clone()), + _ => None, + }) + }; + + let update = build_announce(&route, 4_200_000_001, next_hop, false).unwrap(); + assert_eq!(path(&update, true), Some(vec![4_200_000_001; 2])); + + // What the peer reads from the wire. + let ctx = ParserContext { + four_octet_asn: Some(false), + address_family: None, + }; + let mut buf = BytesMut::new(); + Message::Update(update).to_wire(&ctx, &mut buf).unwrap(); + let (_, Message::Update(seen)) = Message::from_wire(&ctx, &buf).unwrap() else { + panic!("expected UPDATE") + }; + assert_eq!(path(&seen, false), Some(vec![23456; 2]), "AS_TRANS"); + assert_eq!(path(&seen, true), Some(vec![4_200_000_001; 2])); + + // No AS4_PATH when it would say nothing new: the peer handles + // four-octet numbers, or ours fits in two. + let update = build_announce(&route, 4_200_000_001, next_hop, true).unwrap(); + assert_eq!(path(&update, true), None); + let update = build_announce(&route, 65000, next_hop, false).unwrap(); + assert_eq!(path(&update, true), None); + assert_eq!(path(&update, false), Some(vec![65000; 2])); + } + #[test] fn family_mismatch_is_skipped() { let v6 = LocalRoute::new(prefix("2001:db8::/32"), LocalRouteAttrs::default()); - assert!(build_announce(&v6, 65000, IpAddr::V4(Ipv4Addr::LOCALHOST)).is_none()); + assert!(build_announce(&v6, 65000, IpAddr::V4(Ipv4Addr::LOCALHOST), true).is_none()); let v4 = LocalRoute::new(prefix("10.0.0.0/8"), LocalRouteAttrs::default()); - assert!(build_announce(&v4, 65000, IpAddr::V6(Ipv6Addr::LOCALHOST)).is_none()); + assert!(build_announce(&v4, 65000, IpAddr::V6(Ipv6Addr::LOCALHOST), true).is_none()); } #[test] diff --git a/crates/server/src/config.rs b/crates/server/src/config.rs index 50be151..e25dd09 100644 --- a/crates/server/src/config.rs +++ b/crates/server/src/config.rs @@ -8,7 +8,8 @@ use bgp_primitives::ip_prefix::IpPrefix; use bgp_rib::{LocalRoute, LocalRouteAttrs}; use crate::error::{ConfigError, OriginateProblem, PeerProblem}; -use crate::proto::config::{BgpConfig, OriginatedRoute, Peer}; +use crate::family::{AddressFamily, FamilySet}; +use crate::proto::config::{self as proto, BgpConfig, OriginatedRoute, Peer}; const DEFAULT_BGP_PORT: u16 = 179; const DEFAULT_HOLD_TIME_SECS: u16 = 90; @@ -45,6 +46,8 @@ pub struct PeerConfig { /// Source address for connections we open to this peer; `None` lets the /// operating system choose. pub local_address: Option, + /// The address families offered to this peer. + pub families: FamilySet, } impl Config { @@ -153,12 +156,28 @@ fn parse_peer(peer: Peer) -> Result { Some(ip) }; + let families = if peer.address_families.is_empty() { + FamilySet::ALL + } else { + peer.address_families + .iter() + .map(|family| match family.enum_value() { + Ok(proto::AddressFamily::IPV4_UNICAST) => Ok(AddressFamily::Ipv4Unicast), + Ok(proto::AddressFamily::IPV6_UNICAST) => Ok(AddressFamily::Ipv6Unicast), + Ok(proto::AddressFamily::ADDRESS_FAMILY_UNSPECIFIED) | Err(_) => { + Err(PeerProblem::AddressFamily) + } + }) + .collect::>()? + }; + Ok(PeerConfig { name: peer.name, endpoint, remote_asn: peer.remote_asn, hold_time_secs, local_address, + families, }) } @@ -299,6 +318,26 @@ mod tests { ); } + #[test] + fn a_peer_is_offered_every_address_family_unless_some_are_listed() { + let families = |body: &str| { + let config = parse(&format!( + "{BASE}peers {{ name: \"r1\" endpoint {{ address: \"192.0.2.2\" }} remote_asn: 1 {body} }}" + )) + .unwrap(); + config.peers[0].families + }; + assert_eq!(families(""), FamilySet::ALL); + assert_eq!( + families("address_families: IPV6_UNICAST"), + [AddressFamily::Ipv6Unicast].into_iter().collect() + ); + assert_eq!( + families("address_families: IPV4_UNICAST address_families: IPV6_UNICAST"), + FamilySet::ALL + ); + } + #[test] fn bad_peers_and_endpoints_are_rejected_with_the_peer_named() { assert_eq!( @@ -332,6 +371,12 @@ mod tests { problem(&format!("{endpoint} remote_asn: 1 local_address: \"::1\"")), PeerProblem::AddressFamilyMismatch ); + assert_eq!( + problem(&format!( + "{endpoint} remote_asn: 1 address_families: ADDRESS_FAMILY_UNSPECIFIED" + )), + PeerProblem::AddressFamily + ); assert_eq!( problem("endpoint { address: \"192.0.2.2\" port: 70000 } remote_asn: 1"), PeerProblem::Endpoint(Box::new(ConfigError::Port(70000))) diff --git a/crates/server/src/connection.rs b/crates/server/src/connection.rs index 5d4e9ab..ec8b8df 100644 --- a/crates/server/src/connection.rs +++ b/crates/server/src/connection.rs @@ -26,6 +26,7 @@ use tracing::{info, warn}; use crate::collision::{CollisionTable, ConnectionGuard, Direction}; use crate::error::SessionError; +use crate::family::FamilySet; use crate::fsm::{Action, Event, Fsm, FsmConfig, Negotiated}; use crate::metrics::{ MESSAGES_RECEIVED, MESSAGES_SENT, SESSIONS_ESTABLISHED_TOTAL, message_type_name, @@ -53,6 +54,8 @@ pub struct SessionConfig { /// If set, the peer's OPEN must carry this ASN or the session is /// refused with a Bad Peer AS notification. pub expected_asn: Option, + /// The address families to offer the peer. + pub families: FamilySet, } /// Runs a session on an accepted connection and logs how it ended. @@ -79,6 +82,7 @@ pub async fn run_session( local_asn: config.local_asn, hold_time_secs: config.hold_time_secs, expected_asn: config.expected_asn, + families: config.families, }); let mut session = Session { fsm, @@ -204,6 +208,8 @@ impl Session { peer_addr = %self.peer_addr, peer_asn = negotiated.peer_asn, negotiated_hold_time = negotiated.hold_time_secs, + four_octet_asn = negotiated.four_octet_asn, + families = %negotiated.families, direction = self.config.direction.as_str(), "OPEN received, transitioning to OpenConfirm" ); @@ -217,6 +223,13 @@ impl Session { } } Action::Established(negotiated) => self.establish(negotiated), + Action::RoutesIgnored(count) => { + warn!( + peer_addr = %self.peer_addr, + routes = count, + "Ignored routes of an address family that was not negotiated" + ); + } Action::DeliverUpdate(update) => { if let Some(established) = &self.established { let (withdrawn, announced) = prefix_counts(&update); @@ -258,6 +271,7 @@ impl Session { self.config.direction, self.local_addr, negotiated.four_octet_asn, + negotiated.families, outbound_tx, )); self.established = Some(Established { diff --git a/crates/server/src/dialer.rs b/crates/server/src/dialer.rs index 619eb49..ca34f6a 100644 --- a/crates/server/src/dialer.rs +++ b/crates/server/src/dialer.rs @@ -37,6 +37,7 @@ pub fn spawn_dialers( direction: Direction::Outbound, peer_name: Some(peer.name.clone()), expected_asn: Some(peer.remote_asn), + families: peer.families, ..base.clone() }; tokio::spawn(dial_loop(Arc::new(peer.clone()), config, connect_retry)) diff --git a/crates/server/src/error.rs b/crates/server/src/error.rs index fc68fd6..03be11f 100644 --- a/crates/server/src/error.rs +++ b/crates/server/src/error.rs @@ -8,6 +8,7 @@ use bgp_packet::message::NotificationMessage; use bgp_primitives::parser::EncodeError; use thiserror::Error; +use crate::family::FamilySet; use crate::fsm::State; /// Why a session ended abnormally. @@ -38,6 +39,10 @@ pub enum SessionError { WrongPeerAs { expected: u32, actual: u32 }, #[error("closed by connection collision resolution")] Collision, + /// The peer offers none of the address families configured for it, so + /// the session could carry nothing. + #[error("no address family in common: we offer {ours}, the peer offers {theirs}")] + NoCommonAddressFamily { ours: FamilySet, theirs: FamilySet }, } impl SessionError { @@ -63,6 +68,14 @@ impl SessionError { Self::OpenRefused(error) => error.clone(), Self::WrongPeerAs { .. } => BgpError::OpenMsg(OpenMsgSubcode::BadPeerAs), Self::Collision => BgpError::Cease(CeaseSubcode::ConnCollisionResolution), + // RFC 5492 §5: the Data lists the capabilities we need and the + // peer did not offer. + Self::NoCommonAddressFamily { ours, .. } => { + return Some(NotificationMessage { + error: BgpError::OpenMsg(OpenMsgSubcode::UnsupportedCapability), + data: ours.capabilities_wire(), + }); + } }; Some(NotificationMessage { error, @@ -123,6 +136,8 @@ pub enum PeerProblem { LocalAddress(String), #[error("local_address and endpoint are different address families")] AddressFamilyMismatch, + #[error("address_families has a value that is not an address family")] + AddressFamily, } /// What is wrong with one `originate` entry. diff --git a/crates/server/src/family.rs b/crates/server/src/family.rs new file mode 100644 index 0000000..5fa54f5 --- /dev/null +++ b/crates/server/src/family.rs @@ -0,0 +1,357 @@ +//! The address families a session can carry, and which of them it does. +//! +//! Each side lists the families it is willing to exchange in its OPEN, as +//! Multiprotocol capabilities (RFC 4760 §8). A family is in use on a session +//! only if both sides listed it. + +use std::fmt; + +use bgp_packet::message::{PathAttribute, UpdateMessage}; +use bgp_packet::open::{Capability, OpenMessage}; +use bgp_primitives::constants::{AddressFamilyId, SubsequentAfi}; +use bgp_primitives::ip_prefix::IpPrefix; +use bgp_primitives::parser::ParserContext; +use bytes::BytesMut; + +/// An address family this speaker can exchange routes for. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum AddressFamily { + Ipv4Unicast, + Ipv6Unicast, +} + +impl AddressFamily { + pub const ALL: [Self; 2] = [Self::Ipv4Unicast, Self::Ipv6Unicast]; + + pub fn afi(self) -> AddressFamilyId { + match self { + Self::Ipv4Unicast => AddressFamilyId::Ipv4, + Self::Ipv6Unicast => AddressFamilyId::Ipv6, + } + } + + pub fn safi(self) -> SubsequentAfi { + SubsequentAfi::Unicast + } + + fn from_afi_safi(afi: AddressFamilyId, safi: SubsequentAfi) -> Option { + Self::ALL + .into_iter() + .find(|family| family.afi() == afi && family.safi() == safi) + } + + /// The capability that offers this family to a peer. + pub fn capability(self) -> Capability { + Capability::MultiProtocol { + afi: self.afi(), + safi: self.safi(), + } + } + + pub fn name(self) -> &'static str { + match self { + Self::Ipv4Unicast => "IPv4 unicast", + Self::Ipv6Unicast => "IPv6 unicast", + } + } +} + +impl fmt::Display for AddressFamily { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(self.name()) + } +} + +/// A set of address families. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub struct FamilySet { + ipv4_unicast: bool, + ipv6_unicast: bool, +} + +impl FamilySet { + pub const EMPTY: Self = Self { + ipv4_unicast: false, + ipv6_unicast: false, + }; + pub const ALL: Self = Self { + ipv4_unicast: true, + ipv6_unicast: true, + }; + + pub fn insert(&mut self, family: AddressFamily) { + match family { + AddressFamily::Ipv4Unicast => self.ipv4_unicast = true, + AddressFamily::Ipv6Unicast => self.ipv6_unicast = true, + } + } + + pub fn contains(self, family: AddressFamily) -> bool { + match family { + AddressFamily::Ipv4Unicast => self.ipv4_unicast, + AddressFamily::Ipv6Unicast => self.ipv6_unicast, + } + } + + fn contains_afi_safi(self, afi: AddressFamilyId, safi: SubsequentAfi) -> bool { + AddressFamily::from_afi_safi(afi, safi).is_some_and(|family| self.contains(family)) + } + + /// Whether a unicast route for `prefix` may be exchanged. + pub fn carries(self, prefix: &IpPrefix) -> bool { + self.contains_afi_safi(prefix.address_family, SubsequentAfi::Unicast) + } + + pub fn is_empty(self) -> bool { + self == Self::EMPTY + } + + pub fn intersection(self, other: Self) -> Self { + Self { + ipv4_unicast: self.ipv4_unicast && other.ipv4_unicast, + ipv6_unicast: self.ipv6_unicast && other.ipv6_unicast, + } + } + + pub fn iter(self) -> impl Iterator { + AddressFamily::ALL + .into_iter() + .filter(move |family| self.contains(*family)) + } + + /// The families a peer offered in its OPEN. + /// + /// A peer that sends no Multiprotocol capability at all is a speaker of + /// plain RFC 4271 BGP, which carries IPv4 unicast and nothing else. One + /// that sends any has listed everything it wants, IPv4 unicast included. + pub fn offered_by(open: &OpenMessage) -> Self { + let mut listed_any = false; + let mut offered = Self::EMPTY; + for capability in open.capabilities() { + match capability { + Capability::MultiProtocol { afi, safi } => { + listed_any = true; + if let Some(family) = AddressFamily::from_afi_safi(*afi, *safi) { + offered.insert(family); + } + } + // A Multiprotocol capability for a family we have no name + // for is kept as an unknown capability. It still shows that + // the peer lists its families. + Capability::UnknownCapability { type_code: 1, .. } => listed_any = true, + _ => {} + } + } + if listed_any { + offered + } else { + [AddressFamily::Ipv4Unicast].into_iter().collect() + } + } + + /// These families as Multiprotocol capabilities in wire form: the Data + /// of an Unsupported Capability NOTIFICATION (RFC 5492 §5). + pub fn capabilities_wire(self) -> Vec { + let ctx = ParserContext::default(); + let mut out = BytesMut::new(); + for family in self.iter() { + // A Multiprotocol capability always encodes; if that ever + // changed, a NOTIFICATION with less Data is still correct. + if family.capability().to_wire(&ctx, &mut out).is_err() { + return vec![]; + } + } + out.to_vec() + } + + /// Removes from `update` every route of a family that is not in this + /// set, and returns how many prefixes that was. + /// + /// The withdrawn routes and NLRI in the body of an UPDATE are IPv4 + /// unicast; every other family travels in MP_REACH_NLRI and + /// MP_UNREACH_NLRI, which name theirs. + pub fn restrict(self, update: &mut UpdateMessage) -> usize { + let mut removed = 0; + if !self.contains(AddressFamily::Ipv4Unicast) { + removed += update.withdrawn_routes.len() + update.announced_routes.len(); + update.withdrawn_routes.clear(); + update.announced_routes.clear(); + } + update.path_attributes.retain(|attribute| { + let (afi, safi, prefixes) = match attribute { + PathAttribute::MpReachNlri(mp) => (mp.afi, mp.safi, mp.prefixes.len()), + PathAttribute::MpUnreachNlri(mp) => (mp.afi, mp.safi, mp.prefixes.len()), + _ => return true, + }; + let keep = self.contains_afi_safi(afi, safi); + if !keep { + removed += prefixes; + } + keep + }); + removed + } +} + +impl FromIterator for FamilySet { + fn from_iter>(families: I) -> Self { + let mut set = Self::EMPTY; + for family in families { + set.insert(family); + } + set + } +} + +impl fmt::Display for FamilySet { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.is_empty() { + return f.write_str("none"); + } + for (i, family) in self.iter().enumerate() { + if i > 0 { + f.write_str(", ")?; + } + f.write_str(family.name())?; + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use std::net::{Ipv4Addr, Ipv6Addr}; + + use bgp_packet::message::{ + MpReachNlriPathAttribute, MpUnreachNlriPathAttribute, NlriNextHop, OriginPathAttribute, + }; + use bgp_packet::open::OpenOption; + + use super::*; + + fn open(capabilities: Vec) -> OpenMessage { + OpenMessage { + version: 4, + asn: 65001, + hold_time: 90, + identifier: Ipv4Addr::new(10, 0, 0, 2), + options: if capabilities.is_empty() { + vec![] + } else { + vec![OpenOption::Capabilities(capabilities)] + }, + } + } + + fn prefix(s: &str) -> IpPrefix { + IpPrefix::try_from(s).unwrap() + } + + fn only(family: AddressFamily) -> FamilySet { + [family].into_iter().collect() + } + + #[test] + fn a_peer_without_multiprotocol_capabilities_speaks_ipv4_unicast() { + assert_eq!( + FamilySet::offered_by(&open(vec![])), + only(AddressFamily::Ipv4Unicast) + ); + // Other capabilities do not change that. + assert_eq!( + FamilySet::offered_by(&open(vec![Capability::RouteRefresh])), + only(AddressFamily::Ipv4Unicast) + ); + } + + #[test] + fn a_peer_that_lists_families_gets_exactly_those() { + assert_eq!( + FamilySet::offered_by(&open(vec![AddressFamily::Ipv6Unicast.capability()])), + only(AddressFamily::Ipv6Unicast), + "IPv4 unicast is not implied once a peer lists its families" + ); + // Capabilities may be spread over several optional parameters. + let mut spread = open(vec![AddressFamily::Ipv4Unicast.capability()]); + spread.options.push(OpenOption::Capabilities(vec![ + AddressFamily::Ipv6Unicast.capability(), + ])); + assert_eq!(FamilySet::offered_by(&spread), FamilySet::ALL); + // IPv6 multicast is a family, but not one we carry. + assert_eq!( + FamilySet::offered_by(&open(vec![Capability::MultiProtocol { + afi: AddressFamilyId::Ipv6, + safi: SubsequentAfi::Multicast, + }])), + FamilySet::EMPTY + ); + // Nor is one we cannot even name (here L2VPN EVPN, 25/70). + assert_eq!( + FamilySet::offered_by(&open(vec![Capability::UnknownCapability { + type_code: 1, + length: 4, + value: vec![0, 25, 0, 70], + }])), + FamilySet::EMPTY + ); + } + + #[test] + fn sets_intersect_and_describe_themselves() { + let v6 = only(AddressFamily::Ipv6Unicast); + assert_eq!(FamilySet::ALL.intersection(v6), v6); + assert!(only(AddressFamily::Ipv4Unicast).intersection(v6).is_empty()); + assert_eq!(FamilySet::ALL.to_string(), "IPv4 unicast, IPv6 unicast"); + assert_eq!(FamilySet::EMPTY.to_string(), "none"); + assert!(v6.carries(&prefix("2001:db8::/32"))); + assert!(!v6.carries(&prefix("192.0.2.0/24"))); + // One capability each: code 1, length 4, AFI, reserved, SAFI. + assert_eq!( + FamilySet::ALL.capabilities_wire(), + [1, 4, 0, 1, 0, 1, 1, 4, 0, 2, 0, 1] + ); + } + + fn mixed_update() -> UpdateMessage { + UpdateMessage { + withdrawn_routes: vec![prefix("198.51.100.0/24")], + path_attributes: vec![ + PathAttribute::Origin(OriginPathAttribute::IGP), + PathAttribute::MpReachNlri(MpReachNlriPathAttribute { + afi: AddressFamilyId::Ipv6, + safi: SubsequentAfi::Unicast, + next_hop: NlriNextHop::Ipv6(Ipv6Addr::LOCALHOST), + prefixes: vec![prefix("2001:db8:1::/48"), prefix("2001:db8:2::/48")], + }), + PathAttribute::MpUnreachNlri(MpUnreachNlriPathAttribute { + afi: AddressFamilyId::Ipv6, + safi: SubsequentAfi::Unicast, + prefixes: vec![prefix("2001:db8:3::/48")], + }), + ], + announced_routes: vec![prefix("192.0.2.0/24")], + } + } + + #[test] + fn restrict_removes_the_routes_of_families_not_in_the_set() { + let mut update = mixed_update(); + assert_eq!(FamilySet::ALL.restrict(&mut update), 0); + assert_eq!(update, mixed_update()); + + let mut update = mixed_update(); + assert_eq!(only(AddressFamily::Ipv4Unicast).restrict(&mut update), 3); + assert_eq!(update.announced_routes, vec![prefix("192.0.2.0/24")]); + assert_eq!(update.withdrawn_routes, vec![prefix("198.51.100.0/24")]); + assert_eq!( + update.path_attributes, + vec![PathAttribute::Origin(OriginPathAttribute::IGP)] + ); + + let mut update = mixed_update(); + assert_eq!(only(AddressFamily::Ipv6Unicast).restrict(&mut update), 2); + assert!(update.announced_routes.is_empty()); + assert!(update.withdrawn_routes.is_empty()); + assert_eq!(update.path_attributes.len(), 3); + } +} diff --git a/crates/server/src/fsm.rs b/crates/server/src/fsm.rs index 1821759..8c4d3cc 100644 --- a/crates/server/src/fsm.rs +++ b/crates/server/src/fsm.rs @@ -17,9 +17,9 @@ use std::time::Duration; use bgp_packet::errors::{BgpError, DecodeError, OpenMsgSubcode}; use bgp_packet::message::{AS_TRANS, BGP4_VERSION, Message, UpdateMessage}; use bgp_packet::open::{Capability, OpenMessage, OpenOption}; -use bgp_primitives::constants::{AddressFamilyId, SubsequentAfi}; use crate::error::SessionError; +use crate::family::FamilySet; use crate::metrics::message_type_name; /// How long to wait for the peer's OPEN, and for its first KEEPALIVE when @@ -60,6 +60,8 @@ pub struct FsmConfig { pub hold_time_secs: u16, /// If set, the AS the peer must announce in its OPEN. pub expected_asn: Option, + /// The address families to offer the peer. Must not be empty. + pub families: FamilySet, } /// What the two OPENs settled. @@ -71,6 +73,9 @@ pub struct Negotiated { pub hold_time_secs: u16, /// Whether AS numbers are four octets wide on this session (RFC 6793). pub four_octet_asn: bool, + /// The address families both sides offered: the ones routes may be + /// exchanged for. Never empty. + pub families: FamilySet, } /// Something that happened to the connection. @@ -105,6 +110,9 @@ pub enum Action { Admit(Negotiated), /// The session reached Established. Established(Negotiated), + /// The peer sent this many routes of address families that were not + /// negotiated. They have been left out of the update that follows. + RoutesIgnored(usize), /// Routes arrived; hand them to the RIB. DeliverUpdate(UpdateMessage), /// The peer asked for everything we advertise to be sent again. @@ -205,8 +213,15 @@ impl Fsm { actions } (Phase::Established(negotiated), Message::KeepAlive) => heard_from_peer(negotiated), - (Phase::Established(negotiated), Message::Update(update)) => { + (Phase::Established(negotiated), Message::Update(mut update)) => { let mut actions = heard_from_peer(negotiated); + // A route of a family the session does not carry is ignored + // rather than treated as an error: it harms nothing, and + // resetting the session over it would. + let ignored = negotiated.families.restrict(&mut update); + if ignored > 0 { + actions.push(Action::RoutesIgnored(ignored)); + } actions.push(Action::DeliverUpdate(update)); actions } @@ -228,11 +243,9 @@ impl Fsm { } // A four-octet AS number does not fit the OPEN's own field; a peer // that has one puts AS_TRANS there and the number in a capability. - let capability_asn = open.options.iter().find_map(|option| match option { - OpenOption::Capabilities(caps) => caps.iter().find_map(|cap| match cap { - Capability::FourByteAsn { asn } => Some(*asn), - _ => None, - }), + let capability_asn = open.capabilities().find_map(|capability| match capability { + Capability::FourByteAsn { asn } => Some(*asn), + _ => None, }); let peer_asn = capability_asn.unwrap_or(u32::from(open.asn)); if let Some(expected) = self.config.expected_asn @@ -244,11 +257,19 @@ impl Fsm { }); } + let ours = self.config.families; + let theirs = FamilySet::offered_by(&open); + let families = ours.intersection(theirs); + if families.is_empty() { + return self.fail(SessionError::NoCommonAddressFamily { ours, theirs }); + } + let negotiated = Negotiated { peer_asn, peer_identifier: open.identifier, hold_time_secs: self.config.hold_time_secs.min(open.hold_time), four_octet_asn: capability_asn.is_some(), + families, }; self.phase = Phase::OpenConfirm(negotiated); vec![ @@ -313,27 +334,19 @@ fn build_open(config: &FsmConfig) -> Message { // Use AS_TRANS in the 2-byte field when the ASN doesn't fit; the real ASN // is carried in the FourByteAsn capability (RFC 6793). let asn_field = u16::try_from(config.local_asn).unwrap_or(AS_TRANS); + let mut capabilities = vec![Capability::FourByteAsn { + asn: config.local_asn, + }]; + // One Multiprotocol capability per family we will exchange. A peer does + // not accept routes of a family that was not offered (RFC 4760 §8). + capabilities.extend(config.families.iter().map(|family| family.capability())); + capabilities.push(Capability::RouteRefresh); Message::Open(OpenMessage { version: BGP4_VERSION, asn: asn_field, hold_time: config.hold_time_secs, identifier: config.router_id, - options: vec![OpenOption::Capabilities(vec![ - Capability::FourByteAsn { - asn: config.local_asn, - }, - Capability::MultiProtocol { - afi: AddressFamilyId::Ipv4, - safi: SubsequentAfi::Unicast, - }, - // Without this a peer will not accept the IPv6 routes we - // originate via MP_REACH_NLRI (RFC 4760 §8). - Capability::MultiProtocol { - afi: AddressFamilyId::Ipv6, - safi: SubsequentAfi::Unicast, - }, - Capability::RouteRefresh, - ])], + options: vec![OpenOption::Capabilities(capabilities)], }) } @@ -355,9 +368,14 @@ fn validate_open(open: &OpenMessage, local_id: Ipv4Addr) -> Result<(), BgpError> #[cfg(test)] mod tests { use bgp_packet::errors::{CeaseSubcode, FsmSubcode, MsgHeaderSubcode}; - use bgp_packet::message::NotificationMessage; + use bgp_packet::message::{ + MpReachNlriPathAttribute, NlriNextHop, NotificationMessage, PathAttribute, + }; + use bgp_primitives::constants::{AddressFamilyId, SubsequentAfi}; + use bgp_primitives::ip_prefix::IpPrefix; use super::*; + use crate::family::AddressFamily; const LOCAL_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 1); const PEER_ID: Ipv4Addr = Ipv4Addr::new(10, 0, 0, 2); @@ -368,6 +386,18 @@ mod tests { local_asn: 65000, hold_time_secs: 90, expected_asn: None, + families: FamilySet::ALL, + } + } + + fn only(family: AddressFamily) -> FamilySet { + [family].into_iter().collect() + } + + fn capabilities(open: &Message) -> Vec<&Capability> { + match open { + Message::Open(open) => open.capabilities().collect(), + other => panic!("expected OPEN, got {other:?}"), } } @@ -447,6 +477,8 @@ mod tests { peer_identifier: PEER_ID, hold_time_secs: 30, four_octet_asn: false, + // The peer listed no families, so it speaks IPv4 unicast. + families: only(AddressFamily::Ipv4Unicast), }; assert!(matches!( &actions[..], @@ -509,6 +541,102 @@ mod tests { assert!(negotiated.four_octet_asn); } + #[test] + fn our_open_offers_exactly_the_configured_families() { + let (_, actions) = Fsm::new(FsmConfig { + families: only(AddressFamily::Ipv6Unicast), + ..config() + }); + let Action::Send(open) = &actions[0] else { + panic!("expected OPEN first, got {actions:?}") + }; + assert_eq!( + capabilities(open), + [ + &Capability::FourByteAsn { asn: 65000 }, + &AddressFamily::Ipv6Unicast.capability(), + &Capability::RouteRefresh, + ] + ); + } + + #[test] + fn the_session_carries_the_families_both_sides_offered() { + let negotiated = |ours, theirs: Vec| { + let (mut fsm, _) = Fsm::new(FsmConfig { + families: ours, + ..config() + }); + let open = OpenMessage { + options: vec![OpenOption::Capabilities( + theirs.into_iter().map(AddressFamily::capability).collect(), + )], + ..peer_open(90) + }; + let actions = fsm.handle(Event::Message(Message::Open(open))); + (fsm, actions) + }; + + let (fsm, _) = negotiated(FamilySet::ALL, vec![AddressFamily::Ipv6Unicast]); + assert_eq!( + fsm.negotiated().unwrap().families, + only(AddressFamily::Ipv6Unicast) + ); + let (fsm, _) = negotiated( + only(AddressFamily::Ipv4Unicast), + AddressFamily::ALL.to_vec(), + ); + assert_eq!( + fsm.negotiated().unwrap().families, + only(AddressFamily::Ipv4Unicast) + ); + + // Nothing in common: the session could carry no routes at all. The + // NOTIFICATION names the capability we needed. + let (fsm, actions) = negotiated( + only(AddressFamily::Ipv4Unicast), + vec![AddressFamily::Ipv6Unicast], + ); + assert!(matches!( + &actions[0], + Action::Send(Message::Notification(n)) + if n.error == BgpError::OpenMsg(OpenMsgSubcode::UnsupportedCapability) + && n.data == [1, 4, 0, 1, 0, 1] + )); + assert!(matches!( + ending(&fsm, actions).1, + Err(SessionError::NoCommonAddressFamily { .. }) + )); + } + + #[test] + fn routes_of_a_family_that_was_not_negotiated_are_ignored() { + // The peer lists no families, so only IPv4 unicast is in use. + let mut fsm = fsm_in(State::Established, 90); + let v4 = IpPrefix::try_from("192.0.2.0/24").unwrap(); + let update = UpdateMessage { + withdrawn_routes: vec![], + path_attributes: vec![PathAttribute::MpReachNlri(MpReachNlriPathAttribute { + afi: AddressFamilyId::Ipv6, + safi: SubsequentAfi::Unicast, + next_hop: NlriNextHop::Ipv6(std::net::Ipv6Addr::LOCALHOST), + prefixes: vec![IpPrefix::try_from("2001:db8::/32").unwrap()], + })], + announced_routes: vec![v4.clone()], + }; + + let actions = fsm.handle(Event::Message(Message::Update(update))); + assert_eq!(fsm.state(), State::Established, "the session survives"); + assert!(matches!( + &actions[..], + [ + Action::ResetHoldTimer(_), + Action::RoutesIgnored(1), + Action::DeliverUpdate(delivered) + ] if delivered.announced_routes == [v4] && delivered.path_attributes.is_empty() + )); + } + #[test] fn an_unacceptable_open_is_refused_with_the_matching_subcode() { let refused = |open: OpenMessage, config: FsmConfig| { diff --git a/crates/server/src/lib.rs b/crates/server/src/lib.rs index ec48c69..d4f06ff 100644 --- a/crates/server/src/lib.rs +++ b/crates/server/src/lib.rs @@ -4,6 +4,7 @@ pub mod config; pub mod connection; pub mod dialer; pub mod error; +pub mod family; pub mod fsm; pub mod metrics; pub mod server; diff --git a/crates/server/src/server.rs b/crates/server/src/server.rs index 2262f51..8447a04 100644 --- a/crates/server/src/server.rs +++ b/crates/server/src/server.rs @@ -14,6 +14,7 @@ use crate::config::Config; use crate::connection::{SessionConfig, handle_connection}; use crate::dialer::spawn_dialers; use crate::error::ServerError; +use crate::family::FamilySet; use crate::session::{ConfiguredPeer, SessionRegistry}; pub struct BgpServerBuilder { @@ -115,6 +116,7 @@ impl BgpServer { direction: Direction::Inbound, peer_name: None, expected_asn: None, + families: FamilySet::ALL, }; let _advertiser = spawn_advertiser( @@ -138,7 +140,7 @@ impl BgpServer { match result { Ok((stream, peer_addr)) => { // Inbound connections from a configured peer inherit its - // name, expected ASN and hold time. + // name, expected ASN, hold time and address families. let mut config = session_config.clone(); if let Some(peer) = self .config @@ -149,6 +151,7 @@ impl BgpServer { config.peer_name = Some(peer.name.clone()); config.expected_asn = Some(peer.remote_asn); config.hold_time_secs = peer.hold_time_secs; + config.families = peer.families; } tokio::spawn(handle_connection(stream, peer_addr, Arc::new(config))); } diff --git a/crates/server/src/session.rs b/crates/server/src/session.rs index 49f0f77..ed707b6 100644 --- a/crates/server/src/session.rs +++ b/crates/server/src/session.rs @@ -19,6 +19,7 @@ use bgp_rib::PeerId; use tokio::sync::{broadcast, mpsc}; use crate::collision::Direction; +use crate::family::FamilySet; use crate::metrics::SESSIONS_ESTABLISHED; const EVENT_CHANNEL_CAPACITY: usize = 256; @@ -59,6 +60,9 @@ pub struct SessionHandle { /// 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, + /// The address families negotiated with the peer. Routes of any other + /// family are neither sent to it nor accepted from it. + pub families: FamilySet, outbound: mpsc::Sender, } @@ -73,6 +77,7 @@ impl SessionHandle { direction: Direction, local_addr: SocketAddr, four_octet_asn: bool, + families: FamilySet, outbound: mpsc::Sender, ) -> Self { Self { @@ -82,6 +87,7 @@ impl SessionHandle { local_addr, established_at: SystemTime::now(), four_octet_asn, + families, outbound, } } diff --git a/crates/web/src/api.rs b/crates/web/src/api.rs index a405f2f..c2b9436 100644 --- a/crates/web/src/api.rs +++ b/crates/web/src/api.rs @@ -50,6 +50,8 @@ pub struct PeerView { pub established_at_unix: Option, pub uptime_secs: Option, pub four_octet_asn: Option, + /// The address families negotiated with the peer, e.g. `IPv4 unicast`. + pub address_families: Option>, /// Why the last connection attempt or session ended, while in Backoff. pub last_error: Option, pub retry_at_unix: Option, @@ -67,6 +69,7 @@ impl From<&SessionHandle> for PeerView { established_at_unix: Some(unix_secs(s.established_at)), uptime_secs: Some(s.established_at.elapsed().map(|d| d.as_secs()).unwrap_or(0)), four_octet_asn: Some(s.four_octet_asn), + address_families: Some(s.families.iter().map(|f| f.to_string()).collect()), last_error: None, retry_at_unix: None, } @@ -92,6 +95,7 @@ impl PeerView { established_at_unix: None, uptime_secs: None, four_octet_asn: None, + address_families: None, last_error, retry_at_unix, } diff --git a/crates/web/static/statusz.html b/crates/web/static/statusz.html index ceec486..3eb0f4d 100644 --- a/crates/web/static/statusz.html +++ b/crates/web/static/statusz.html @@ -57,7 +57,7 @@ View: variables, status json, <

Peers (...)

- +
namepeerASstateopened bylocal address (next hop)establisheduptime4-byte ASdetail
namepeerASstateopened bylocal address (next hop)establisheduptime4-byte ASaddress familiesdetail
@@ -137,7 +137,7 @@ View: variables, status json, < function renderPeers() { const b = $('peers-body'); $('peers-count').textContent = `${state.peers.filter(p => p.state === 'Established').length} of ${state.peers.length} established`; - if (!state.peers.length) { setHTML(b, 'No peers configured and none connected.'); return; } + if (!state.peers.length) { setHTML(b, 'No peers configured and none connected.'); return; } setHTML(b, state.peers.map(p => { const up = p.state === 'Established'; const detail = p.state === 'Backoff' ? `${esc(p.last_error || '')}; retry in ${until(p.retry_at_unix)}` : ''; @@ -145,7 +145,8 @@ View: variables, status json, < ${p.name ? esc(p.name) : '(inbound)'}${esc(p.peer_addr)}${p.peer_asn} ${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}${detail}`; + ${up ? age(p.established_at_unix) : DASH}${up ? (p.four_octet_asn ? 'yes' : 'no') : DASH} + ${up ? esc((p.address_families || []).join(', ')) : DASH}${detail}`; }).join('')); } diff --git a/testdata/server.textpb b/testdata/server.textpb index 4fa3d2b..63dfe35 100644 --- a/testdata/server.textpb +++ b/testdata/server.textpb @@ -14,6 +14,9 @@ peers { endpoint { address: "127.0.0.1" port: 1791 } remote_asn: 65001 hold_time_seconds: 30 + # Offer only these families to the peer. Left out, all are offered. + address_families: IPV4_UNICAST + address_families: IPV6_UNICAST } connect_retry_seconds: 5