The role capability only works on ebgp sessions. It makes no sense on
ibgp sessions and the RFC 9234 does not define any behaviour for that.
I decided to:
 - Exclude the role capability for ibgp sessions when sending an OPEN
 - Warn when a role capability is received on an iBGP session
 - Make sure the capability negotiation is skipped for ibgp sessions,
   this disables the role capability on the session.

I kept the peer capability intact so that it shows in bgpctl show nei
output.

This diff also includes the necessary bit in the notification handling for
capabilities (I forgot to add CAPA_ROLE there in the initial diff).
-- 
:wq Claudio

Index: session.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/session.c,v
retrieving revision 1.436
diff -u -p -r1.436 session.c
--- session.c   18 Oct 2022 12:24:51 -0000      1.436
+++ session.c   27 Dec 2022 16:41:29 -0000
@@ -1441,8 +1441,8 @@ session_open(struct peer *p)
        if (p->capa.ann.refresh)        /* no data */
                errs += session_capa_add(opb, CAPA_REFRESH, 0);
 
-       /* BGP open policy, RFC 9234 */
-       if (p->capa.ann.role_ena) {
+       /* BGP open policy, RFC 9234, only for ebgp sessions */
+       if (p->capa.ann.role_ena && p->conf.ebgp) {
                errs += session_capa_add(opb, CAPA_ROLE, 1);
                errs += ibuf_add(opb, &p->capa.ann.role, 1);
        }
@@ -2478,6 +2478,11 @@ parse_notification(struct peer *peer)
                                log_peer_warnx(&peer->conf,
                                    "disabling route refresh capability");
                                break;
+                       case CAPA_ROLE:
+                               peer->capa.ann.role_ena = 0;
+                               log_peer_warnx(&peer->conf,
+                                   "disabling role capability");
+                               break;
                        case CAPA_RESTART:
                                peer->capa.ann.grestart.restart = 0;
                                log_peer_warnx(&peer->conf,
@@ -2616,10 +2621,13 @@ parse_capabilities(struct peer *peer, u_
                case CAPA_ROLE:
                        if (capa_len != 1) {
                                log_peer_warnx(&peer->conf,
-                                   "Bad open policy capability length: "
+                                   "Bad role capability length: "
                                    "%u", capa_len);
                                break;
                        }
+                       if (!peer->conf.ebgp)
+                               log_peer_warnx(&peer->conf,
+                                   "Received role capability on iBGP session");
                        peer->capa.peer.role_ena = 1;
                        peer->capa.peer.role = *capa_val;
                        break;
@@ -2835,8 +2843,10 @@ capa_neg_calc(struct peer *p, uint8_t *s
         * Make sure that the roles match and set the negotiated capability
         * to the role of the peer. So the RDE can inject the OTC attribute.
         * See RFC 9234, section 4.2.
+        * These check should only happen on ebgp sessions.
         */
-       if (p->capa.ann.role_ena != 0 && p->capa.peer.role_ena != 0) {
+       if (p->capa.ann.role_ena != 0 && p->capa.peer.role_ena != 0 &&
+           p->conf.ebgp) {
                switch (p->capa.ann.role) {
                case CAPA_ROLE_PROVIDER:
                        if (p->capa.peer.role != CAPA_ROLE_CUSTOMER)
@@ -2868,7 +2878,7 @@ capa_neg_calc(struct peer *p, uint8_t *s
                }
                p->capa.neg.role_ena = 1;
                p->capa.neg.role = p->capa.peer.role;
-       } else if (p->capa.ann.role_ena == 2) {
+       } else if (p->capa.ann.role_ena == 2 && p->conf.ebgp) {
                /* enforce presence of open policy role capability */
                log_peer_warnx(&p->conf, "open policy role enforced but "
                    "not present");

Reply via email to