]> git.puffer.fish Git - matthieu/frr.git/commitdiff
bgpd: Fix errors handling for MP/GR capabilities as dynamic capability
authorDonatas Abraitis <donatas@opensourcerouting.org>
Sat, 30 Mar 2024 13:35:18 +0000 (15:35 +0200)
committerMergify <37929162+mergify[bot]@users.noreply.github.com>
Fri, 5 Apr 2024 17:45:22 +0000 (17:45 +0000)
When receiving a MP/GR capability as dynamic capability, but malformed, do not
forget to advance the pointer to avoid hitting infinity loop.

After:
```
Mar 29 11:15:28 donatas-laptop bgpd[353550]: [GS0AQ-HKY0X] 127.0.0.1 rcv CAPABILITY
Mar 29 11:15:28 donatas-laptop bgpd[353550]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 5, length 0
Mar 29 11:15:28 donatas-laptop bgpd[353550]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 0, length 0
Mar 29 11:15:28 donatas-laptop bgpd[353550]: [HFHDS-QT71N][EC 33554494] 127.0.0.1(donatas-pc): unrecognized capability code: 0 - ignored
Mar 29 11:15:28 donatas-laptop bgpd[353550]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 0, code: 0, length 0
Mar 29 11:15:28 donatas-laptop bgpd[353550]: [HFHDS-QT71N][EC 33554494] 127.0.0.1(donatas-pc): unrecognized capability code: 0 - ignored
Mar 29 11:15:28 donatas-laptop bgpd[353550]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 0, code: 0, length 0
Mar 29 11:15:28 donatas-laptop bgpd[353550]: [HFHDS-QT71N][EC 33554494] 127.0.0.1(donatas-pc): unrecognized capability code: 0 - ignored
Mar 29 11:15:28 donatas-laptop bgpd[353550]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 0, code: 0, length 1
Mar 29 11:15:28 donatas-laptop bgpd[353550]: [HFHDS-QT71N][EC 33554494] 127.0.0.1(donatas-pc): unrecognized capability code: 0 - ignored
Mar 29 11:15:28 donatas-laptop bgpd[353550]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:15:28 donatas-laptop bgpd[353550]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
```

Before:
```
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [Z1DRQ-N6Z5F] 127.0.0.1(donatas-pc): Dynamic Capability MultiProtocol Extensions afi/safi invalid (bad-value/unicast)
Mar 29 11:14:54 donatas-laptop bgpd[347675]: [JTVED-VGTQQ] 127.0.0.1(donatas-pc): CAPABILITY has action: 1, code: 1, length 10
```

Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit 30a332dad86fafd2b0b6c61d23de59ed969a219b)

bgpd/bgp_packet.c

index cae82cbbb77fdd1dbe20ccd02cec3cee3c3ded80..50e5b54abbe738eee47dae9a8560ff58db869093 100644 (file)
@@ -3121,6 +3121,7 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
                        zlog_err("%pBP: Capability length error", peer);
                        bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE,
                                        BGP_NOTIFY_SUBCODE_UNSPECIFIC);
+                       pnt += length;
                        return BGP_Stop;
                }
                action = *pnt;
@@ -3133,7 +3134,7 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
                                 action);
                        bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE,
                                        BGP_NOTIFY_SUBCODE_UNSPECIFIC);
-                       return BGP_Stop;
+                       goto done;
                }
 
                if (bgp_debug_neighbor_events(peer))
@@ -3145,12 +3146,13 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
                        zlog_err("%pBP: Capability length error", peer);
                        bgp_notify_send(peer->connection, BGP_NOTIFY_CEASE,
                                        BGP_NOTIFY_SUBCODE_UNSPECIFIC);
+                       pnt += length;
                        return BGP_Stop;
                }
 
                /* Ignore capability when override-capability is set. */
                if (CHECK_FLAG(peer->flags, PEER_FLAG_OVERRIDE_CAPABILITY))
-                       continue;
+                       goto done;
 
                capability = lookup_msg(capcode_str, hdr->code, "Unknown");
 
@@ -3165,7 +3167,7 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
                                         peer, capability,
                                         sizeof(struct capability_mp_data),
                                         hdr->length);
-                               return BGP_Stop;
+                               goto done;
                        }
 
                        memcpy(&mpc, pnt + 3, sizeof(struct capability_mp_data));
@@ -3180,7 +3182,7 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
                                                   peer, capability,
                                                   iana_afi2str(pkt_afi),
                                                   iana_safi2str(pkt_safi));
-                               continue;
+                               goto done;
                        }
 
                        /* Address family check.  */
@@ -3207,7 +3209,7 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
                                if (peer_active_nego(peer))
                                        bgp_clear_route(peer, afi, safi);
                                else
-                                       return BGP_Stop;
+                                       goto done;
                        }
                        break;
                case CAPABILITY_CODE_RESTART:
@@ -3217,7 +3219,7 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
                                bgp_notify_send(peer->connection,
                                                BGP_NOTIFY_CEASE,
                                                BGP_NOTIFY_SUBCODE_UNSPECIFIC);
-                               return BGP_Stop;
+                               goto done;
                        }
 
                        bgp_dynamic_capability_graceful_restart(pnt, action,
@@ -3243,7 +3245,7 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
                                bgp_notify_send(peer->connection,
                                                BGP_NOTIFY_CEASE,
                                                BGP_NOTIFY_SUBCODE_UNSPECIFIC);
-                               return BGP_Stop;
+                               goto done;
                        }
 
                        uint8_t role;
@@ -3265,6 +3267,7 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
                        break;
                }
 
+done:
                pnt += hdr->length + 3;
        }