bgpd: avoid BGP port opening for non default VRF instance #18962
Conversation
bgpd/bgp_vty.c
Outdated
| /* | ||
| * Check if there is no neighbors nor listening range on bgp | ||
| */ | ||
| static int bgp_may_stop_listening(struct bgp *bgp, struct vty *vty) |
There was a problem hiding this comment.
You don't use return code anywhere, is this expected?
There was a problem hiding this comment.
taken into account
bgpd/bgp_vty.c
Outdated
| } | ||
|
|
||
|
|
||
| static int bgp_need_listening(struct bgp *bgp, struct vty *vty) |
There was a problem hiding this comment.
For this function you also don't check return code.
There was a problem hiding this comment.
taken into account
| @@ -4784,6 +4842,10 @@ DEFUN (bgp_listen_range, | |||
| return CMD_WARNING_CONFIG_FAILED; | |||
| } | |||
|
|
|||
| /* if need start listening */ | |||
| if (bgp->inst_type != BGP_INSTANCE_TYPE_DEFAULT && bgp->name) | |||
There was a problem hiding this comment.
This condition is checked inside bgp_need_listening(), right?
| @@ -3934,7 +3934,9 @@ int bgp_get(struct bgp **bgp_val, as_t *as, const char *name, | |||
| /* BGP server socket already processed if BGP instance | |||
| * already part of the list | |||
| */ | |||
| bgp_handle_socket(bgp, vrf, VRF_UNKNOWN, true); | |||
| if (bgp->inst_type != BGP_INSTANCE_TYPE_VRF) | |||
There was a problem hiding this comment.
BGP_INSTANCE_TYPE_VIEW no need to be checked?
There was a problem hiding this comment.
no in this case no new vrf creater
fdumontet6WIND
force-pushed
the
ports
branch
2 times, most recently
from
ac06d53 to
74e3881
Compare
bgpd/bgp_vty.c
Outdated
| /* | ||
| * if need stop listening | ||
| */ | ||
| if ((ret == CMD_SUCCESS) && bgp_may_stop_listening(bgp, vty) != CMD_SUCCESS) |
There was a problem hiding this comment.
Doing:
ret = bgp_may_stop_listening(bgp, vty)wouldn't be enough?
There was a problem hiding this comment.
taken into account
| ! | ||
| interface r3-eth1 | ||
| ! | ||
| ip route 192.0.2.1/32 172.31.10.1router bgp 64500 view one |
There was a problem hiding this comment.
taken into account
|
|
||
| for rname, router in router_list.items(): | ||
| logger.info("Loading router %s" % rname) | ||
| router.load_frr_config(os.path.join(CWD, "{}/zebra.conf".format(rname))) |
There was a problem hiding this comment.
taken into account
| def check_port_179_open(): | ||
| tgen = get_topogen() | ||
| r1 = tgen.gears["r1"] | ||
| output = r1.cmd("ss -tuplen | grep 179 ") |
There was a problem hiding this comment.
This check is not correct. Either we should change to ss -ntl | grep ":179 ", or use a regexp, because with your current check it can match not only a port, but e.g. inode number, etc.or source port,
There was a problem hiding this comment.
the regex is made 2 line under:
re.search("r1-cust", output)
| def build_topo(tgen): | ||
| "Build function" | ||
|
|
||
| # Create 7 PE routers. |
| assertmsg = "BGP port related to r1-cust not listening when a neighbor is set twice" | ||
| assert res is not None, assertmsg | ||
|
|
||
| step("remove the neighbor from r1-cust twice") |
There was a problem hiding this comment.
What does it mean twice in this context?
There was a problem hiding this comment.
it's the second time that this command occurs
fdumontet6WIND
force-pushed
the
ports
branch
3 times, most recently
from
679d364 to
d8eac62
Compare
Refactor `bgp_need_listening()` to prevent unnecessary reactivation of BGP listening sockets for non default vrf instances that are already active. This function now checks whether a listener is already associated with the BGP instance before attempting to open a new socket. Also modify `bgp_get()` to skip socket setup for VRF instances, which are now handled more appropriately elsewhere based on configuration changes like `neighbor` or `listen-range`. Signed-off-by: Francois Dumontet <[email protected]>
Introduce a helper function `bgp_may_stop_listening()` to check whether a BGP instance of type VRF has no remaining neighbors or listen ranges. If so, the listening socket is closed accordingly. This logic is now invoked after `no neighbor` and `no bgp listen-range` commands, allowing BGP to release the listening socket when no longer needed, improving security and VRF isolation. Signed-off-by: Francois Dumontet <[email protected]>
|
ci:rerun same test fails on other pull request |
|
ci:rerun |
verify that BGP port are open/close when needed fo non default VRF Signed-off-by: Francois Dumontet <[email protected]>
Currently bgpd open a listening port for every existing bgp instance.
This leads to have open port (identified as a security leak) for l3vrf without neigbhors.
Now only default VRF is opening is BGP port at starting
other VRF wait for neigbhor creation or listening-range command