Skip to content

Commit f0206ca

Browse files
committed
bgp: more validations for route leaks-RFC 9234
Add BGP leak coverage mechanisms as specified in RFC 9234 Chapter 5. A few hacks added, since this requires a lot of interaction of both the attrs on the BaseAttr and the 'remote_role' field in Neighbor, some adjustments had to be made. Such hacks have been represented by 'FIXIT:' keyword. These should be removed after this commit. Signed-off-by: Paul Wekesa <paul1tw1@gmail.com>
1 parent f6e9041 commit f0206ca

6 files changed

Lines changed: 139 additions & 19 deletions

File tree

holo-bgp/src/af.rs

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
use std::net::{IpAddr, Ipv4Addr, Ipv6Addr};
88

9-
use holo_utils::bgp::AfiSafi;
9+
use holo_utils::bgp::{AfiSafi, RoleName};
1010
use holo_utils::ip::{IpAddrKind, IpNetworkKind, Ipv4AddrExt, Ipv6AddrExt};
1111
use ipnetwork::{Ipv4Network, Ipv6Network};
1212
use itertools::Itertools;
@@ -52,7 +52,10 @@ pub trait AddressFamily: Sized {
5252
fn nexthop_tx_change(nbr: &Neighbor, local: bool, attrs: &mut BaseAttrs);
5353

5454
// Build BGP UPDATE messages based on the provided update queue.
55-
fn build_updates(queue: &mut NeighborUpdateQueue<Self>) -> Vec<Message>;
55+
fn build_updates(
56+
queue: &mut NeighborUpdateQueue<Self>,
57+
role: Option<RoleName>,
58+
) -> Vec<Message>;
5659
}
5760

5861
#[derive(Debug)]
@@ -122,13 +125,33 @@ impl AddressFamily for Ipv4Unicast {
122125
}
123126
}
124127

125-
fn build_updates(queue: &mut NeighborUpdateQueue<Self>) -> Vec<Message> {
128+
fn build_updates(
129+
queue: &mut NeighborUpdateQueue<Self>,
130+
role: Option<RoleName>,
131+
) -> Vec<Message> {
126132
let mut msgs = vec![];
127133
let reach = std::mem::take(&mut queue.reach);
128134
let unreach = std::mem::take(&mut queue.unreach);
129135

130136
// Reachable prefixes.
131137
for (attrs, prefixes) in reach.into_iter() {
138+
// RFC 9234: If a route already contains the OTC Attribute,
139+
// it MUST NOT be propagated to Providers, Peers,or RSes.
140+
//
141+
// FIXIT: This logic was placed here as best fit. It necessitated
142+
// the addition of 'role' argument to the function, which may /
143+
// may not be necessary. If a better place can be found
144+
//
145+
if let Some(role) = role
146+
&& matches!(
147+
role,
148+
RoleName::Provider | RoleName::Peer | RoleName::Rs
149+
)
150+
&& attrs.base.otc.is_some()
151+
{
152+
continue;
153+
}
154+
132155
let nexthop = Ipv4Addr::get(attrs.base.nexthop.unwrap()).unwrap();
133156
let max = (Message::MAX_LEN
134157
- UpdateMsg::MIN_LEN
@@ -255,13 +278,32 @@ impl AddressFamily for Ipv6Unicast {
255278
}
256279
}
257280

258-
fn build_updates(queue: &mut NeighborUpdateQueue<Self>) -> Vec<Message> {
281+
fn build_updates(
282+
queue: &mut NeighborUpdateQueue<Self>,
283+
role: Option<RoleName>,
284+
) -> Vec<Message> {
259285
let mut msgs = vec![];
260286
let reach = std::mem::take(&mut queue.reach);
261287
let unreach = std::mem::take(&mut queue.unreach);
262288

263289
// Reachable prefixes.
264290
for (attrs, prefixes) in reach.into_iter() {
291+
// RFC 9234: If a route already contains the OTC Attribute,
292+
// it MUST NOT be propagated to Providers, Peers,or RSes.
293+
//
294+
// FIXIT: This logic was placed here as best fit. It necessitated
295+
// the addition of 'role' argument to the function, which may /
296+
// may not be necessary. If a better place can be found
297+
if let Some(role) = role
298+
&& matches!(
299+
role,
300+
RoleName::Provider | RoleName::Peer | RoleName::Rs
301+
)
302+
&& attrs.base.otc.is_some()
303+
{
304+
continue;
305+
}
306+
265307
let nexthop = Ipv6Addr::get(attrs.base.nexthop.unwrap()).unwrap();
266308
let ll_nexthop = attrs.base.ll_nexthop;
267309
let nexthop_len = if ll_nexthop.is_some() { 32 } else { 16 };

holo-bgp/src/events.rs

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::net::IpAddr;
88

99
use chrono::Utc;
1010
use holo_protocol::InstanceShared;
11-
use holo_utils::bgp::RouteType;
11+
use holo_utils::bgp::{RoleName, RouteType};
1212
use holo_utils::ibus::IbusChannelsTx;
1313
use holo_utils::ip::{IpAddrKind, IpNetworkKind};
1414
use holo_utils::policy::{PolicyResult, PolicyType};
@@ -285,6 +285,19 @@ fn process_nbr_reach_prefixes<A>(
285285
return;
286286
}
287287

288+
// RFC 9234: If a route is received from a Provider, a Peer or an RS and
289+
// the OTC Attribute is not present, it MUST be added with AS number of
290+
// remote AS.
291+
//
292+
// FIXIT: Confirmation for if the following check can be placed anywhere
293+
// better should be done.
294+
if let Some(role) = nbr.remote_role
295+
&& matches!(role, RoleName::Provider | RoleName::Peer | RoleName::Rs)
296+
&& attrs.base.otc.is_none()
297+
{
298+
let _ = attrs.base.otc.get_or_insert(nbr.config.peer_as);
299+
}
300+
288301
// Initialize route origin and type.
289302
let origin = RouteOrigin::Neighbor {
290303
identifier: nbr.identifier.unwrap(),
@@ -411,7 +424,8 @@ fn process_nbr_route_refresh(
411424
}
412425

413426
// Send UPDATE message(s) to the neighbor.
414-
let msg_list = nbr.update_queues.build_updates();
427+
let remote_role = nbr.remote_role;
428+
let msg_list = nbr.update_queues.build_updates(remote_role);
415429
if !msg_list.is_empty() {
416430
nbr.message_list_send(msg_list);
417431
}
@@ -587,7 +601,8 @@ where
587601
}
588602

589603
// Send UPDATE message(s) to the neighbor.
590-
let msg_list = nbr.update_queues.build_updates();
604+
let remote_role = nbr.remote_role;
605+
let msg_list = nbr.update_queues.build_updates(remote_role);
591606
if !msg_list.is_empty() {
592607
nbr.message_list_send(msg_list);
593608
}
@@ -679,6 +694,7 @@ where
679694
let best_route = rib::best_path::<A>(
680695
dest,
681696
instance.config.asn,
697+
neighbors,
682698
&table.nht,
683699
selection_cfg,
684700
);
@@ -793,7 +809,8 @@ fn withdraw_routes<A>(
793809
}
794810

795811
// Send UPDATE message(s) to the neighbor.
796-
let msg_list = nbr.update_queues.build_updates();
812+
let remote_role = nbr.remote_role;
813+
let msg_list = nbr.update_queues.build_updates(remote_role);
797814
if !msg_list.is_empty() {
798815
nbr.message_list_send(msg_list);
799816
}

holo-bgp/src/neighbor.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1075,7 +1075,8 @@ impl Neighbor {
10751075
// Re-send the current Adj-RIB-Out to this neighbor.
10761076
self.resend_adj_rib_out::<Ipv4Unicast>(instance);
10771077
self.resend_adj_rib_out::<Ipv6Unicast>(instance);
1078-
let msg_list = self.update_queues.build_updates();
1078+
let msg_list =
1079+
self.update_queues.build_updates(self.remote_role);
10791080
if !msg_list.is_empty() {
10801081
self.message_list_send(msg_list);
10811082
}
@@ -1197,10 +1198,13 @@ impl MessageStatistics {
11971198
// ===== impl NeighborUpdateQueues =====
11981199

11991200
impl NeighborUpdateQueues {
1200-
pub(crate) fn build_updates(&mut self) -> Vec<Message> {
1201+
pub(crate) fn build_updates(
1202+
&mut self,
1203+
role: Option<RoleName>,
1204+
) -> Vec<Message> {
12011205
[
1202-
self.ipv4_unicast.build_updates(),
1203-
self.ipv6_unicast.build_updates(),
1206+
self.ipv4_unicast.build_updates(role),
1207+
self.ipv6_unicast.build_updates(role),
12041208
]
12051209
.concat()
12061210
}
@@ -1212,8 +1216,8 @@ impl<A> NeighborUpdateQueue<A>
12121216
where
12131217
A: AddressFamily,
12141218
{
1215-
fn build_updates(&mut self) -> Vec<Message> {
1216-
A::build_updates(self)
1219+
fn build_updates(&mut self, role: Option<RoleName>) -> Vec<Message> {
1220+
A::build_updates(self, role)
12171221
}
12181222
}
12191223

holo-bgp/src/northbound/yang.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ impl ToYang for RouteIneligibleReason {
207207
RouteIneligibleReason::Originator => "iana-bgp-rib-types:ineligible-originator".into(),
208208
RouteIneligibleReason::Confed => "iana-bgp-rib-types:ineligible-confed".into(),
209209
RouteIneligibleReason::Unresolvable => "holo-bgp:ineligible-unresolvable".into(),
210+
RouteIneligibleReason::Role => "holo-bgp:ineligible-role".into(),
210211
}
211212
}
212213
}

holo-bgp/src/rib.rs

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use std::net::{IpAddr, Ipv4Addr};
1010
use std::sync::Arc;
1111
use std::time::Instant;
1212

13-
use holo_utils::bgp::RouteType;
13+
use holo_utils::bgp::{RoleName, RouteType};
1414
use holo_utils::ibus::IbusChannelsTx;
1515
use holo_utils::protocol::Protocol;
1616
use prefix_trie::map::PrefixMap;
@@ -19,7 +19,7 @@ use serde::{Deserialize, Serialize};
1919
use crate::af::{AddressFamily, Ipv4Unicast, Ipv6Unicast};
2020
use crate::debug::Debug;
2121
use crate::ibus;
22-
use crate::neighbor::{Neighbor, PeerType};
22+
use crate::neighbor::{Neighbor, Neighbors, PeerType};
2323
use crate::northbound::configuration::{
2424
DistanceCfg, InstanceTraceOptions, MultipathCfg, RouteSelectionCfg,
2525
};
@@ -145,6 +145,7 @@ pub enum RouteIneligibleReason {
145145
Originator,
146146
Confed,
147147
Unresolvable,
148+
Role,
148149
}
149150

150151
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
@@ -689,6 +690,7 @@ where
689690
pub(crate) fn best_path<A>(
690691
dest: &mut Destination,
691692
local_asn: u32,
693+
neighbors: &Neighbors,
692694
nht: &HashMap<IpAddr, NhtEntry<A>>,
693695
selection_cfg: &RouteSelectionCfg,
694696
) -> Option<Box<Route>>
@@ -698,7 +700,7 @@ where
698700
let mut best_route = None;
699701

700702
// Iterate over each Adj-RIB-In route for the destination.
701-
for route in dest
703+
'rib_loop: for route in dest
702704
.adj_rib
703705
.values_mut()
704706
// Pick the post-policy routes.
@@ -712,7 +714,37 @@ where
712714
// First, check if the route is eligible.
713715
if route.attrs.base.value.as_path.contains(local_asn) {
714716
route.ineligible_reason = Some(RouteIneligibleReason::AsLoop);
715-
continue;
717+
continue 'rib_loop;
718+
}
719+
720+
// Check for RoleName and OTC validity.
721+
// RFC 9234 Section 5.
722+
//
723+
// 1.
724+
// If a route with the OTC Attribute is received from a Customer or an
725+
// RS-Client, then it is a route leak and be considered ineligible.
726+
//
727+
// 2.
728+
// If a route with the OTC Attribute is received from a Peer
729+
// (i.e., remote AS with a Peer Role) and the Attribute has
730+
// a value that is not equal to the remote (i.e., Peer's) AS number,
731+
// then it is a route leak and be considered ineligible.
732+
if let RouteOrigin::Neighbor { remote_addr, .. } = route.origin
733+
&& let Some(nbr) = neighbors.get(&remote_addr)
734+
&& let Some(role) = nbr.remote_role
735+
{
736+
if let Some(otc) = route.attrs.base.value.otc {
737+
let is_leak = match role {
738+
RoleName::Customer | RoleName::RsClient => true,
739+
RoleName::Peer if otc != nbr.config.peer_as => true,
740+
_ => false,
741+
};
742+
743+
if is_leak {
744+
route.ineligible_reason = Some(RouteIneligibleReason::Role);
745+
continue 'rib_loop;
746+
}
747+
}
716748
}
717749

718750
// Get interior cost to the route's nexthop.
@@ -722,7 +754,7 @@ where
722754
if route.igp_cost.is_none() {
723755
route.ineligible_reason =
724756
Some(RouteIneligibleReason::Unresolvable);
725-
continue;
757+
continue 'rib_loop;
726758
};
727759
}
728760

@@ -851,6 +883,22 @@ pub(crate) fn attrs_tx_update<A>(
851883

852884
// Remove the LOCAL_PREF attribute.
853885
attrs.base.local_pref = None;
886+
887+
// RFC 9234 - Section 5:
888+
// If a route is to be advertised to a Customer, a Peer, or an
889+
// RS-Client (when the sender is an RS)
890+
// and the OTC Attribute is not present, then when advertising the
891+
// route, an OTC Attribute
892+
// be added with a value equal to the AS number of the local AS
893+
if let Some(remote_role) = nbr.remote_role
894+
&& matches!(
895+
remote_role,
896+
RoleName::Customer | RoleName::Peer | RoleName::Rs
897+
)
898+
&& attrs.base.otc.is_none()
899+
{
900+
attrs.base.otc = Some(local_asn);
901+
}
854902
}
855903
}
856904

holo-yang/modules/augmentations/holo-bgp.yang

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,14 @@ module holo-bgp {
4040
"Route was ineligible due to unresolvable next-hop";
4141
}
4242

43+
identity ineligible-role {
44+
base brt:ineligible-route-reason;
45+
description
46+
"Route's OTC configuration was not compatible with neighbor.";
47+
reference
48+
"RFC 9234: Bgp Route Leak Prevention.";
49+
}
50+
4351
identity unknown-error {
4452
base bn:bgp-notification;
4553
description

0 commit comments

Comments
 (0)