]> git.puffer.fish Git - mirror/frr.git/commitdiff
bgpd: Fix errors handling for MP/GR capabilities as dynamic capability 15640/head
authorDonatas Abraitis <donatas@opensourcerouting.org>
Sat, 30 Mar 2024 13:35:18 +0000 (15:35 +0200)
committerDonatas Abraitis <donatas@opensourcerouting.org>
Sat, 30 Mar 2024 14:15:08 +0000 (16:15 +0200)
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>
bgpd/bgp_packet.c

index 78878013f8de4b856d3f8c8c0b2921bd0d879b09..78554893ff152d644dbe1a95a1260322e64f3f40 100644 (file)
@@ -3734,6 +3734,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;
@@ -3746,7 +3747,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))
@@ -3758,12 +3759,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");
 
@@ -3778,7 +3780,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));
@@ -3793,7 +3795,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.  */
@@ -3820,7 +3822,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:
@@ -3830,7 +3832,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,
@@ -3866,7 +3868,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;
@@ -3888,6 +3890,7 @@ static int bgp_capability_msg_parse(struct peer *peer, uint8_t *pnt,
                        break;
                }
 
+done:
                pnt += hdr->length + 3;
        }