RFC5492 is fairly explicit when a capability should be enabled on a
session:
A BGP speaker that supports a particular capability may use this
capability with its peer after the speaker determines (as described
above) that the peer supports this capability. Simply put, a given
capability can be used on a peering if that capability has been
advertised by both peers. If either peer has not advertised it, the
capability cannot be used.
Adjust capa_neg_calc() to follow this strict model.
This affects route refersh and graceful restart. For graceful restart this
requires to flush the RIBs immediatly.
Also ignore and warn about RREFRESH messages that are received on a
session where route refesh is disabled.
--
:wq Claudio
Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.414
diff -u -p -r1.414 session.c
--- session.c 6 May 2021 09:18:54 -0000 1.414
+++ session.c 12 May 2021 16:33:42 -0000
@@ -1636,7 +1636,7 @@ session_neighbor_rrefresh(struct peer *p
{
u_int8_t i;
- if (!p->capa.peer.refresh)
+ if (!p->capa.neg.refresh)
return (-1);
for (i = 0; i < AID_MAX; i++) {
@@ -2257,6 +2257,11 @@ parse_refresh(struct peer *peer)
return (0);
}
+ if (!peer->capa.neg.refresh) {
+ log_peer_warnx(&peer->conf, "peer sent unexpected refresh");
+ return (0);
+ }
+
if (imsg_rde(IMSG_REFRESH, peer->conf.id, &aid, sizeof(aid)) == -1)
return (-1);
@@ -2546,16 +2551,15 @@ capa_neg_calc(struct peer *p)
{
u_int8_t i, hasmp = 0;
- /* refresh: does not realy matter here, use peer setting */
- p->capa.neg.refresh = p->capa.peer.refresh;
+ /* a capability is accepted only if both sides announced it */
- /* as4byte: both side must announce capability */
- if (p->capa.ann.as4byte && p->capa.peer.as4byte)
- p->capa.neg.as4byte = 1;
- else
- p->capa.neg.as4byte = 0;
+ p->capa.neg.refresh =
+ (p->capa.ann.refresh && p->capa.peer.refresh) != 0;
+
+ p->capa.neg.as4byte =
+ (p->capa.ann.as4byte && p->capa.peer.as4byte) != 0;
- /* MP: both side must announce capability */
+ /* MP: both side must agree on the AFI,SAFI pair */
for (i = 0; i < AID_MAX; i++) {
if (p->capa.ann.mp[i] && p->capa.peer.mp[i])
p->capa.neg.mp[i] = 1;
@@ -2569,18 +2573,21 @@ capa_neg_calc(struct peer *p)
p->capa.neg.mp[AID_INET] = 1;
/*
- * graceful restart: only the peer capabilities are of interest here.
+ * graceful restart: the peer capabilities are of interest here.
* It is necessary to compare the new values with the previous ones
* and act acordingly. AFI/SAFI that are not part in the MP capability
* are treated as not being present.
+ * Also make sure that a flush happens if the session stopped
+ * supporting graceful restart.
*/
for (i = 0; i < AID_MAX; i++) {
int8_t negflags;
/* disable GR if the AFI/SAFI is not present */
- if (p->capa.peer.grestart.flags[i] & CAPA_GR_PRESENT &&
- p->capa.neg.mp[i] == 0)
+ if (p->capa.ann.grestart.restart == 0 ||
+ (p->capa.peer.grestart.flags[i] & CAPA_GR_PRESENT &&
+ p->capa.neg.mp[i] == 0))
p->capa.peer.grestart.flags[i] = 0; /* disable */
/* look at current GR state and decide what to do */
negflags = p->capa.neg.grestart.flags[i];
@@ -2600,6 +2607,8 @@ capa_neg_calc(struct peer *p)
}
p->capa.neg.grestart.timeout = p->capa.peer.grestart.timeout;
p->capa.neg.grestart.restart = p->capa.peer.grestart.restart;
+ if (p->capa.ann.grestart.restart == 0)
+ p->capa.neg.grestart.restart = 0;
return (0);
}
