bgp: add more role validation

Make sure the OTC is added before route is sent to Customer, peer or
RS-Client. Additional check for if route is locally originated has
been added.

Move the RoleName to iana file.

Signed-off-by: Paul Wekesa <paul1tw1@gmail.com>

Author: Paul-weqe

holo-bgp/src/events.rs

@@ -22,7 +22,7 @@ use crate::error::{Error, IoError, NbrRxError};
 use crate::instance::{InstanceUpView, PolicyApplyTasks};
 use crate::neighbor::{Neighbor, Neighbors, PeerType, fsm};
 use crate::packet::attribute::Attrs;
-use crate::packet::iana::{Afi, Safi};
+use crate::packet::iana::{Afi, RoleName, Safi};
 use crate::packet::message::{
     Capability, Message, MpReachNlri, MpUnreachNlri, RouteRefreshMsg, UpdateMsg,
 };
@@ -285,6 +285,18 @@ fn process_nbr_reach_prefixes<A>(
         return;
     }
 
+    // RFC 9234: If a route is received from a Provider, a Peer or an RS and
+    // the OTC Attribute is not present, it MUST be added with AS number of
+    // remote AS.
+    if let Some(remote_role) = nbr.remote_role
+        && matches!(
+            remote_role,
+            RoleName::Provider | RoleName::Peer | RoleName::Rs
+        )
+    {
+        let _ = attrs.base.otc.get_or_insert(nbr.config.peer_as);
+    }
+
     // Initialize route origin and type.
     let origin = RouteOrigin::Neighbor {
         identifier: nbr.identifier.unwrap(),
@@ -564,6 +576,20 @@ where
 
                     // Update route's attributes before transmission.
                     let mut attrs = rpinfo.attrs;
+
+                    // RFC 9234; If a route already contains the OTC Attribute,
+                    // it be propagated to Providers, Peers, or RSes.
+                    if let Some(remote_role) = nbr.remote_role
+                        && matches!(
+                            remote_role,
+                            RoleName::Provider | RoleName::Peer | RoleName::Rs
+                        )
+                        && attrs.base.otc.is_some()
+                        && !rpinfo.origin.is_local()
+                    {
+                        continue;
+                    }
+
                     rib::attrs_tx_update::<A>(
                         &mut attrs,
                         nbr,
@@ -679,6 +705,7 @@ where
         let best_route = rib::best_path::<A>(
             dest,
             instance.config.asn,
+            neighbors,
             &table.nht,
             selection_cfg,
         );

holo-bgp/src/neighbor.rs

@@ -13,7 +13,7 @@ use std::time::Duration;
 use arbitrary::Arbitrary;
 use chrono::{DateTime, Utc};
 use holo_protocol::InstanceChannelsTx;
-use holo_utils::bgp::{AfiSafi, RoleName, RouteType, WellKnownCommunities};
+use holo_utils::bgp::{AfiSafi, RouteType, WellKnownCommunities};
 use holo_utils::ibus::IbusChannelsTx;
 use holo_utils::socket::{TTL_MAX, TcpConnInfo, TcpStream};
 use holo_utils::task::{IntervalTask, Task, TimeoutTask};
@@ -30,7 +30,7 @@ use crate::northbound::notification;
 use crate::northbound::rpc::ClearType;
 use crate::packet::attribute::{AS_TRANS, Attrs};
 use crate::packet::iana::{
-    Afi, CeaseSubcode, ErrorCode, FsmErrorSubcode, Safi,
+    Afi, CeaseSubcode, ErrorCode, FsmErrorSubcode, RoleName, Safi,
 };
 use crate::packet::message::{
     Capability, DecodeCxt, EncodeCxt, KeepaliveMsg, Message,
@@ -698,7 +698,7 @@ impl Neighbor {
             });
         }
 
-        if let Some(role) = self.config.role {
+        if let Some(role) = self.config.local_role {
             capabilities.insert(Capability::Role { role });
         }
 
@@ -814,40 +814,46 @@ impl Neighbor {
             ));
         }
 
-        if let Some(local_role) = self.config.role
+        // RFC 9234: Role correctness validation for OPEN messages.
+        let mut role_correctness = true;
+        if let Some(local_role) = self.config.local_role
             && let Some(Capability::Role { role: remote_role }) = msg
                 .capabilities
                 .iter()
                 .find(|cap| matches!(cap, Capability::Role { .. }))
         {
-            let err = Err(Error::NbrRoleMismatch(
-                self.remote_addr,
-                local_role.to_u8().unwrap(),
-                remote_role.to_u8().unwrap(),
-            ));
-
-            // Validate the incoming BGP Role.
-            // ---
-            // Validation 1:
-            // Check if the neighbor had already sent a Role capability,
-            // and if the role capability is same as incoming.
-            if self.remote_role.is_some()
-                && self.remote_role != Some(*remote_role)
+            // If remote role already exists, it should be same as incoming role.
+            if let Some(r_role) = self.remote_role
+                && r_role != *remote_role
             {
-                return err;
+                // TODO: (RFC 9234 Section 4.2)
+                // Use error below only when 'strict mode'
+                // is enabled
+                //
+                // let err = Err(Error::NbrRoleMismatch(
+                //     self.remote_addr,
+                //     local_role.to_u8().unwrap(),
+                //     remote_role.to_u8().unwrap(),
+                // ));
+                // Should throw error on struct mode.
+                role_correctness = false;
             }
 
-            // Validation 2:
-            // Finds if:
-            //  1. Role has been locally configured.
-            //  2. role exists on the incoming message.
-            //  3. If local role and remote role correctly match the RFC 9234.
-            if !RoleName::validate_role_correctness(&local_role, remote_role) {
-                return err;
+            // Valid Role Mappings.
+            let role_mappings = BTreeMap::from([
+                (RoleName::Provider, RoleName::Customer),
+                (RoleName::Customer, RoleName::Provider),
+                (RoleName::Rs, RoleName::RsClient),
+                (RoleName::RsClient, RoleName::Rs),
+                (RoleName::Peer, RoleName::Peer),
+            ]);
+
+            if let Some(approved_role) = role_mappings.get(&local_role)
+                && approved_role == remote_role
+                && role_correctness
+            {
+                self.remote_role = Some(*remote_role);
             }
-
-            // If everything is okay, then we can set the remote role correctly.
-            self.remote_role = Some(*remote_role);
         }
 
         Ok(())

holo-bgp/src/northbound/configuration.rs

@@ -13,7 +13,7 @@ use std::sync::{Arc, LazyLock as Lazy};
 use arc_swap::ArcSwap;
 use enum_as_inner::EnumAsInner;
 use holo_northbound::configuration::{Callbacks, CallbacksBuilder, Provider, ValidationCallbacks, ValidationCallbacksBuilder};
-use holo_utils::bgp::{AfiSafi, RoleName};
+use holo_utils::bgp::AfiSafi;
 use holo_utils::ip::{AddressFamily, IpAddrKind};
 use holo_utils::policy::{ApplyPolicyCfg, DefaultPolicyType};
 use holo_utils::protocol::Protocol;
@@ -25,7 +25,7 @@ use crate::instance::{Instance, InstanceUpView};
 use crate::neighbor::{Neighbor, PeerType, fsm};
 use crate::network;
 use crate::northbound::yang_gen::bgp;
-use crate::packet::iana::{CeaseSubcode, ErrorCode};
+use crate::packet::iana::{CeaseSubcode, ErrorCode, RoleName};
 use crate::packet::message::{Message, NotificationMsg};
 use crate::rib::RouteOrigin;
 
@@ -136,7 +136,7 @@ pub struct NeighborCfg {
     pub prefix_limit: PrefixLimitCfg,
     pub afi_safi: BTreeMap<AfiSafi, NeighborAfiSafiCfg>,
     pub trace_opts: NeighborTraceOptions,
-    pub role: Option<RoleName>,
+    pub local_role: Option<RoleName>,
 }
 
 #[derive(Debug)]
@@ -253,6 +253,26 @@ fn load_callbacks() -> Callbacks<Instance> {
         .path(bgp::global::PATH)
         .create_apply(|_instance, _args| {})
         .delete_apply(|_instance, _args| {})
+        .path(bgp::neighbors::neighbor::local_role::PATH)
+        .modify_apply(|instance, args| {
+            let nbr_addr = args.list_entry.into_neighbor().unwrap();
+            let nbr = instance.neighbors.get_mut(&nbr_addr).unwrap();
+
+            let local_role = args.dnode.get_string();
+            nbr.config.local_role = match local_role.as_str() {
+                "provider" => Some(RoleName::Provider),
+                "customer" => Some(RoleName::Customer),
+                "peer" => Some(RoleName::Peer),
+                "rs-client" => Some(RoleName::RsClient),
+                "rs" => Some(RoleName::Rs),
+                _ => None,
+            };
+        })
+        .delete_apply(|instance, args| {
+            let nbr_addr = args.list_entry.into_neighbor().unwrap();
+            let nbr = instance.neighbors.get_mut(&nbr_addr).unwrap();
+            nbr.config.local_role = None;
+        })
         .path(bgp::global::r#as::PATH)
         .modify_apply(|instance, args| {
             let asn = args.dnode.get_u32();
@@ -1778,7 +1798,7 @@ impl Default for NeighborCfg {
             prefix_limit: Default::default(),
             afi_safi: Default::default(),
             trace_opts: Default::default(),
-            role: Default::default(),
+            local_role: Default::default(),
         }
     }
 }

holo-bgp/src/northbound/yang.rs

@@ -208,6 +208,7 @@ impl ToYang for RouteIneligibleReason {
             RouteIneligibleReason::Originator => "iana-bgp-rib-types:ineligible-originator".into(),
             RouteIneligibleReason::Confed => "iana-bgp-rib-types:ineligible-confed".into(),
             RouteIneligibleReason::Unresolvable => "holo-bgp:ineligible-unresolvable".into(),
+            RouteIneligibleReason::Role => "holo-bgp:ineligible-role".into(),
         }
     }
 }

holo-bgp/src/packet/attribute.rs

@@ -908,7 +908,7 @@ mod otc {
     const LEN: u8 = 4;
 
     pub(super) fn encode(otc: u32, buf: &mut BytesMut) {
-        buf.put_u8(AttrFlags::OPTIONAL.bits());
+        buf.put_u8(AttrFlags::OPTIONAL.bits() | AttrFlags::TRANSITIVE.bits());
         buf.put_u8(AttrType::Otc as u8);
         buf.put_u8(LEN);
         buf.put_u32(otc);

holo-bgp/src/packet/iana.rs

@@ -289,6 +289,18 @@ pub enum AttrType {
     //AttrSet = 128,
 }
 
+// Roles as defined in RFC 9234.
+#[derive(Clone, Copy, Ord, Debug, Eq, Hash, PartialEq, PartialOrd)]
+#[derive(FromPrimitive, ToPrimitive)]
+#[derive(Deserialize, Serialize)]
+pub enum RoleName {
+    Provider = 0,
+    Rs = 1,
+    RsClient = 2,
+    Customer = 3,
+    Peer = 4, // i.e Lateral Peer.
+}
+
 // BGP Origin.
 pub type Origin = holo_utils::bgp::Origin;
 

holo-bgp/src/packet/message.rs

@@ -10,7 +10,6 @@ use std::net::{Ipv4Addr, Ipv6Addr};
 use arbitrary::Arbitrary;
 use bytes::{Buf, BufMut, Bytes, BytesMut};
 use enum_as_inner::EnumAsInner;
-use holo_utils::bgp::RoleName;
 use holo_utils::bytes::{BytesExt, BytesMutExt, TLS_BUF};
 use holo_utils::ip::{
     Ipv4AddrExt, Ipv4NetworkExt, Ipv6AddrExt, Ipv6NetworkExt,
@@ -28,7 +27,8 @@ use crate::packet::error::{
 };
 use crate::packet::iana::{
     Afi, CapabilityCode, ErrorCode, MessageHeaderErrorSubcode, MessageType,
-    OpenMessageErrorSubcode, OpenParamType, Safi, UpdateMessageErrorSubcode,
+    OpenMessageErrorSubcode, OpenParamType, RoleName, Safi,
+    UpdateMessageErrorSubcode,
 };
 
 //

holo-bgp/src/rib.rs

@@ -19,13 +19,14 @@ use serde::{Deserialize, Serialize};
 use crate::af::{AddressFamily, Ipv4Unicast, Ipv6Unicast};
 use crate::debug::Debug;
 use crate::ibus;
-use crate::neighbor::{Neighbor, PeerType};
+use crate::neighbor::{Neighbor, Neighbors, PeerType};
 use crate::northbound::configuration::{
     DistanceCfg, InstanceTraceOptions, MultipathCfg, RouteSelectionCfg,
 };
 use crate::packet::attribute::{
     Attrs, BaseAttrs, Comms, ExtComms, Extv6Comms, LargeComms, UnknownAttr,
 };
+use crate::packet::iana::RoleName;
 use crate::policy::RoutePolicyInfo;
 
 // Default values.
@@ -145,6 +146,7 @@ pub enum RouteIneligibleReason {
     Originator,
     Confed,
     Unresolvable,
+    Role,
 }
 
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
@@ -689,6 +691,7 @@ where
 pub(crate) fn best_path<A>(
     dest: &mut Destination,
     local_asn: u32,
+    neighbors: &Neighbors,
     nht: &HashMap<IpAddr, NhtEntry<A>>,
     selection_cfg: &RouteSelectionCfg,
 ) -> Option<Box<Route>>
@@ -698,7 +701,7 @@ where
     let mut best_route = None;
 
     // Iterate over each Adj-RIB-In route for the destination.
-    for route in dest
+    'rib_loop: for route in dest
         .adj_rib
         .values_mut()
         // Pick the post-policy routes.
@@ -712,7 +715,39 @@ where
         // First, check if the route is eligible.
         if route.attrs.base.value.as_path.contains(local_asn) {
             route.ineligible_reason = Some(RouteIneligibleReason::AsLoop);
-            continue;
+            continue 'rib_loop;
+        }
+
+        // Check for RoleName and OTC validity.
+        // RFC 9234 Section 5.
+        //
+        // 1.
+        // If a route with the OTC Attribute is received from a Customer or an
+        // RS-Client, then it is a route leak and be considered ineligible.
+        //
+        // 2.
+        // If a route with the OTC Attribute is received from a Peer
+        // (i.e., remote AS with a Peer Role) and the Attribute has
+        // a value that is not equal to the remote (i.e., Peer's) AS number,
+        // then it is a route leak and be considered ineligible.
+        if let RouteOrigin::Neighbor { remote_addr, .. } = route.origin
+            && let Some(nbr) = neighbors.get(&remote_addr)
+            && let Some(remote_role) = nbr.remote_role
+        {
+            let attrs = &route.attrs.get();
+
+            if let Some(otc) = attrs.base.otc {
+                let is_leak = match remote_role {
+                    RoleName::Customer | RoleName::RsClient => true,
+                    RoleName::Peer if otc != nbr.config.peer_as => true,
+                    _ => false,
+                };
+
+                if is_leak {
+                    route.ineligible_reason = Some(RouteIneligibleReason::Role);
+                    continue 'rib_loop;
+                }
+            }
         }
 
         // Get interior cost to the route's nexthop.
@@ -722,7 +757,7 @@ where
             if route.igp_cost.is_none() {
                 route.ineligible_reason =
                     Some(RouteIneligibleReason::Unresolvable);
-                continue;
+                continue 'rib_loop;
             };
         }
 
@@ -851,6 +886,22 @@ pub(crate) fn attrs_tx_update<A>(
 
             // Remove the LOCAL_PREF attribute.
             attrs.base.local_pref = None;
+
+            if let Some(remote_role) = nbr.remote_role {
+                // RFC 9234 - Section 5:
+                // If a route is to be advertised to a Customer, a Peer, or an
+                // RS-Client (when the sender is an RS) and the OTC Attribute is
+                // not present, then when advertising the route, an OTC Attribute
+                // be added with a value equal to the AS number of the local AS
+                match (remote_role, nbr.config.local_role) {
+                    (RoleName::Customer, _)
+                    | (RoleName::Peer, _)
+                    | (RoleName::RsClient, Some(RoleName::Rs)) => {
+                        attrs.base.otc.get_or_insert(local_asn);
+                    }
+                    (_, _) => {}
+                }
+            }
         }
     }