Skip to content

Commit 693d31b

Browse files
committed
bgpd: Enforce RFC 4271 Section 5.1.3 - disallow NEXT_HOP equal to peer address
A locally originated route must not be advertised to a peer using that peer's own address as NEXT_HOP. After outbound route-map processing, if the final NEXT_HOP equals the receiving peer's address, reset it to the local address to comply with RFC 4271 Section 5.1.3 and prevent a blackhole. Fixes #22600 Signed-off-by: Sri Mohan <sri.mohan@samsung.com>
1 parent 4c4f482 commit 693d31b

9 files changed

Lines changed: 269 additions & 4 deletions

File tree

bgpd/bgp_errors.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,12 @@ static struct log_ref ferr_bgp_err[] = {
581581
.description = "Failed to parse an Unreachability Information SAFI (SAFI 81) NLRI received from a peer. The NLRI may have been truncated, contained malformed TLVs, or violated the wire format described in draft-tantsura-idr-unreachability-safi.",
582582
.suggestion = "Verify the sending peer implements draft-tantsura-idr-unreachability-safi correctly. Note that error logs for this EC are rate-limited to prevent log flooding from malicious or misbehaving peers.",
583583
},
584+
{
585+
.code = EC_BGP_NEXTHOP_SELF_PEER,
586+
.title = "Outbound NEXT_HOP equals receiving peer address",
587+
.description = "A locally originated route's final NEXT_HOP, after route-map processing, equals the receiving peer's own address. This violates RFC 4271 Section 5.1.3. The NEXT_HOP has been reset to the local address.",
588+
.suggestion = "Review outbound route-map configuration to avoid setting the next-hop to the receiving peer's address.",
589+
},
584590
{
585591
.code = END_FERR,
586592
}

bgpd/bgp_errors.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ enum bgp_log_refs {
108108
EC_BGP_TTL_SECURITY_FAIL,
109109
EC_BGP_LS_PACKET,
110110
EC_BGP_UNREACH_PARSE_FAILURE,
111+
EC_BGP_NEXTHOP_SELF_PEER,
111112
};
112113

113114
extern void bgp_error_init(void);

bgpd/bgp_route.c

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3120,8 +3120,61 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi,
31203120
if (p->family == AF_INET6 || peer_cap_enhe(peer, afi, safi)) {
31213121
if (IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_global)) {
31223122
subgroup_announce_reset_nhop(AF_INET6, attr);
3123-
nh_reset = true;
3124-
}
3123+
nh_reset = true;
3124+
}
3125+
}
3126+
3127+
/*
3128+
* RFC 4271 Section 5.1.3:
3129+
* "A route originated by a BGP speaker SHALL NOT be
3130+
* advertised to a peer using an address of that peer
3131+
* as NEXT_HOP."
3132+
*
3133+
* After route-map processing, the NEXT_HOP may have been
3134+
* set (e.g., via "set ip next-hop") to the receiving
3135+
* peer's own address. Reset it to self to comply with
3136+
* the RFC and avoid a blackhole. This must happen before
3137+
* the WECMP/link-bandwidth and AIGP adjustments below,
3138+
* which are conditioned on nh_reset being true.
3139+
*/
3140+
if (from == bgp->peer_self && peer->connection && peer->connection->su_remote) {
3141+
if (afi == AFI_IP && safi == SAFI_UNICAST &&
3142+
sockunion_family(peer->connection->su_remote) == AF_INET &&
3143+
IPV4_ADDR_SAME(&attr->nexthop, &peer->connection->su_remote->sin.sin_addr)) {
3144+
flog_warn(EC_BGP_NEXTHOP_SELF_PEER,
3145+
"%s: %pBP [Update:SEND] %pFX NEXT_HOP %pI4 equals peer address, resetting to self (RFC 4271 5.1.3)",
3146+
__func__, peer, p, &attr->nexthop);
3147+
subgroup_announce_reset_nhop(AF_INET, attr);
3148+
/*
3149+
* Clear the route-map nexthop change flags so the
3150+
* update formation code (bpacket_reformat_for_peer)
3151+
* fills in our local address instead of leaving the
3152+
* nexthop as 0.0.0.0.
3153+
*/
3154+
UNSET_FLAG(attr->rmap_change_flags, BATTR_RMAP_IPV4_NHOP_CHANGED);
3155+
UNSET_FLAG(attr->rmap_change_flags, BATTR_RMAP_VPNV4_NHOP_CHANGED);
3156+
UNSET_FLAG(attr->rmap_change_flags, BATTR_RMAP_NEXTHOP_PEER_ADDRESS);
3157+
nh_reset = true;
3158+
} else if (NEXTHOP_IS_V6 &&
3159+
sockunion_family(peer->connection->su_remote) == AF_INET6 &&
3160+
IPV6_ADDR_SAME(&attr->mp_nexthop_global,
3161+
&peer->connection->su_remote->sin6.sin6_addr)) {
3162+
flog_warn(EC_BGP_NEXTHOP_SELF_PEER,
3163+
"%s: %pBP [Update:SEND] %pFX NEXT_HOP %pI6 equals peer address, resetting to self (RFC 4271 5.1.3)",
3164+
__func__, peer, p, &attr->mp_nexthop_global);
3165+
subgroup_announce_reset_nhop(AF_INET6, attr);
3166+
/*
3167+
* Clear the route-map nexthop change flags so the
3168+
* update formation code (bpacket_reformat_for_peer)
3169+
* fills in our local address instead of leaving the
3170+
* nexthop as ::.
3171+
*/
3172+
UNSET_FLAG(attr->rmap_change_flags, BATTR_RMAP_IPV6_GLOBAL_NHOP_CHANGED);
3173+
UNSET_FLAG(attr->rmap_change_flags, BATTR_RMAP_VPNV6_GLOBAL_NHOP_CHANGED);
3174+
UNSET_FLAG(attr->rmap_change_flags, BATTR_RMAP_IPV6_LL_NHOP_CHANGED);
3175+
UNSET_FLAG(attr->rmap_change_flags, BATTR_RMAP_NEXTHOP_PEER_ADDRESS);
3176+
nh_reset = true;
3177+
}
31253178
}
31263179

31273180
/* If this is an iBGP, send Origin Validation State (OVS)

bgpd/bgp_routemap.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2335,11 +2335,22 @@ route_set_ip_nexthop(void *rule, const struct prefix *prefix, void *object)
23352335
/* Set next hop value. */
23362336
bgp_attr_set(path->attr, BGP_ATTR_NEXT_HOP);
23372337
path->attr->nexthop = *rins->address;
2338-
SET_FLAG(path->attr->rmap_change_flags,
2339-
BATTR_RMAP_IPV4_NHOP_CHANGED);
23402338
/* case for MP-BGP : MPLS VPN */
23412339
path->attr->mp_nexthop_global_in = *rins->address;
23422340
path->attr->mp_nexthop_len = sizeof(*rins->address);
2341+
2342+
/*
2343+
* RFC 4271 Section 5.1.3: If the configured nexthop equals
2344+
* the peer's own address, mark it so the packet rewrite code
2345+
* can correct it to the local address (prevent routing blackhole).
2346+
*/
2347+
if (peer->connection->su_remote &&
2348+
sockunion_family(peer->connection->su_remote) == AF_INET &&
2349+
peer->connection->su_remote->sin.sin_addr.s_addr == rins->address->s_addr) {
2350+
SET_FLAG(path->attr->rmap_change_flags, BATTR_RMAP_NEXTHOP_PEER_ADDRESS);
2351+
} else {
2352+
SET_FLAG(path->attr->rmap_change_flags, BATTR_RMAP_IPV4_NHOP_CHANGED);
2353+
}
23432354
}
23442355

23452356
return RMAP_OKAY;

bgpd/bgp_updgrp_packet.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,9 @@ struct stream *bpacket_reformat_for_peer(struct bpacket *pkt,
416416
BPKT_ATTRVEC_FLAGS_RMAP_NH_PEER_ADDRESS)) {
417417
mod_v4nh = &peer->nexthop.v4;
418418
nh_modified = 1;
419+
flog_warn(EC_BGP_NEXTHOP_SELF_PEER,
420+
"%s: Route advertised to %s has NEXT_HOP set to peer's own address (%pI4), resetting to local address (%pI4) per RFC 4271 Section 5.1.3",
421+
__func__, peer->host, &v4nh, mod_v4nh);
419422
}
420423
} else if (v4nh.s_addr == INADDR_ANY) {
421424
mod_v4nh = &peer->nexthop.v4;
@@ -510,6 +513,9 @@ struct stream *bpacket_reformat_for_peer(struct bpacket *pkt,
510513
BPKT_ATTRVEC_FLAGS_RMAP_NH_PEER_ADDRESS)) {
511514
mod_v6nhg = &peer->nexthop.v6_global;
512515
gnh_modified = 1;
516+
flog_warn(EC_BGP_NEXTHOP_SELF_PEER,
517+
"%s: Route advertised to %s has IPv6 NEXT_HOP set to peer's own address (%pI6), resetting to local address (%pI6) per RFC 4271 Section 5.1.3",
518+
__func__, peer->host, &v6nhglobal, mod_v6nhg);
513519
}
514520
} else if (IN6_IS_ADDR_UNSPECIFIED(&v6nhglobal)) {
515521
mod_v6nhg = &peer->nexthop.v6_global;

tests/topotests/bgp_ipv4_nexthop_peer_self/__init__.py

Whitespace-only changes.
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
!
2+
interface r1-eth0
3+
ip address 10.0.12.1/24
4+
!
5+
ip prefix-list NH_PEER_NET permit 192.0.2.0/24
6+
ip prefix-list NH_NORMAL_NET permit 198.51.100.0/24
7+
!
8+
route-map RM_OUT permit 10
9+
match ip address prefix-list NH_PEER_NET
10+
set ip next-hop 10.0.12.2
11+
!
12+
route-map RM_OUT permit 20
13+
match ip address prefix-list NH_NORMAL_NET
14+
set ip next-hop 10.0.0.1
15+
!
16+
route-map RM_OUT permit 100
17+
!
18+
router bgp 65001
19+
bgp router-id 1.1.1.1
20+
no bgp ebgp-requires-policy
21+
neighbor 10.0.12.2 remote-as 65002
22+
!
23+
address-family ipv4 unicast
24+
network 192.0.2.0/24
25+
network 198.51.100.0/24
26+
neighbor 10.0.12.2 activate
27+
neighbor 10.0.12.2 route-map RM_OUT out
28+
exit-address-family
29+
!
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
!
2+
interface r2-eth0
3+
ip address 10.0.12.2/24
4+
!
5+
router bgp 65002
6+
bgp router-id 2.2.2.2
7+
no bgp ebgp-requires-policy
8+
bgp disable-ebgp-connected-route-check
9+
neighbor 10.0.12.1 remote-as 65001
10+
!
11+
address-family ipv4 unicast
12+
neighbor 10.0.12.1 activate
13+
exit-address-family
14+
!
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
#!/usr/bin/env python
2+
# SPDX-License-Identifier: ISC
3+
4+
#
5+
# Copyright (c) 2025 by Cline SR
6+
#
7+
8+
"""
9+
Test RFC 4271 Section 5.1.3 conformance:
10+
"A route originated by a BGP speaker SHALL NOT be
11+
advertised to a peer using an address of that peer
12+
as NEXT_HOP."
13+
14+
This test verifies that when an outbound route-map uses
15+
"set ip next-hop <peer-address>" on a locally originated route, the
16+
NEXT_HOP is reset to the local address rather than left as the peer's
17+
own address (which would cause a blackhole).
18+
19+
r1 advertises two prefixes to r2:
20+
- 192.0.2.0/24 with NEXT_HOP set to r2's address (10.0.12.2)
21+
=> must be reset to 10.0.12.1 (r1's address)
22+
- 198.51.100.0/24 with NEXT_HOP set to a non-peer address (10.0.0.1)
23+
=> must be left unchanged (no regression)
24+
"""
25+
26+
import os
27+
import sys
28+
import json
29+
import functools
30+
import pytest
31+
32+
CWD = os.path.dirname(os.path.realpath(__file__))
33+
sys.path.append(os.path.join(CWD, "../"))
34+
35+
# pylint: disable=C0413
36+
from lib import topotest
37+
from lib.topogen import Topogen, get_topogen
38+
39+
pytestmark = [pytest.mark.bgpd]
40+
41+
42+
def build_topo(tgen):
43+
"""Build a simple two-router topology: r1 <---> r2"""
44+
r1 = tgen.add_router("r1")
45+
r2 = tgen.add_router("r2")
46+
47+
switch = tgen.add_switch("s1")
48+
switch.add_link(r1)
49+
switch.add_link(r2)
50+
51+
52+
def setup_module(mod):
53+
tgen = Topogen(build_topo, mod.__name__)
54+
tgen.start_topology()
55+
56+
for router in tgen.routers().values():
57+
router.load_frr_config()
58+
59+
tgen.start_router()
60+
61+
62+
def teardown_module(mod):
63+
tgen = get_topogen()
64+
tgen.stop_topology()
65+
66+
67+
def test_bgp_next_hop_not_peer_address():
68+
"""
69+
r1 advertises 192.0.2.0/24 to r2 with an outbound route-map that
70+
sets the NEXT_HOP to r2's address (10.0.12.2). Per RFC 4271 §5.1.3,
71+
the NEXT_HOP must be reset to r1's local address (10.0.12.1),
72+
not left as 10.0.12.2.
73+
"""
74+
tgen = get_topogen()
75+
76+
if tgen.routers_have_failure():
77+
pytest.skip(tgen.errors)
78+
79+
r2 = tgen.gears["r2"]
80+
81+
def _bgp_converge():
82+
output = json.loads(
83+
r2.vtysh_cmd("show bgp ipv4 unicast 192.0.2.0/24 json")
84+
)
85+
expected = {
86+
"routes": {
87+
"192.0.2.0/24": [
88+
{"valid": True, "nexthops": [{"ip": "10.0.12.1"}]}
89+
]
90+
}
91+
}
92+
return topotest.json_cmp(output, expected)
93+
94+
test_func = functools.partial(_bgp_converge)
95+
_, result = topotest.run_and_expect(test_func, None, count=30, wait=1)
96+
assert result is None, (
97+
"192.0.2.0/24 NEXT_HOP is not 10.0.12.1; RFC 4271 5.1.3 violation"
98+
)
99+
100+
101+
def test_bgp_next_hop_normal_route_map():
102+
"""
103+
Verify that a normal route-map setting NEXT_HOP to a non-peer address
104+
still works correctly (no regression). 198.51.100.0/24 should have
105+
NEXT_HOP 10.0.0.1.
106+
"""
107+
tgen = get_topogen()
108+
109+
if tgen.routers_have_failure():
110+
pytest.skip(tgen.errors)
111+
112+
r2 = tgen.gears["r2"]
113+
114+
def _bgp_converge():
115+
output = json.loads(
116+
r2.vtysh_cmd("show bgp ipv4 unicast 198.51.100.0/24 json")
117+
)
118+
expected = {
119+
"routes": {
120+
"198.51.100.0/24": [
121+
{"valid": True, "nexthops": [{"ip": "10.0.0.1"}]}
122+
]
123+
}
124+
}
125+
return topotest.json_cmp(output, expected)
126+
127+
test_func = functools.partial(_bgp_converge)
128+
_, result = topotest.run_and_expect(test_func, None, count=30, wait=1)
129+
assert result is None, (
130+
"198.51.100.0/24 NEXT_HOP is not 10.0.0.1 (regression)"
131+
)
132+
133+
134+
def test_memory_leak():
135+
"Run the memory leak test and report results."
136+
tgen = get_topogen()
137+
if not tgen.is_memleak_enabled():
138+
pytest.skip("Memory leak test/report is disabled")
139+
140+
tgen.report_memory_leaks()
141+
142+
143+
if __name__ == "__main__":
144+
args = ["-s"] + sys.argv[1:]
145+
sys.exit(pytest.main(args))

0 commit comments

Comments
 (0)