BGP: KEEPALIVE with length != 19 is accepted instead of rejected with Bad Message Length
Hello BIRD maintainers, I would like to report a possible issue in BIRD. ### Summary BIRD appears to accept BGP KEEPALIVE messages whose BGP message length is greater than 19 bytes. RFC 4271 defines KEEPALIVE as only the fixed BGP message header, so a valid KEEPALIVE has length 19 and no body. In dynamic testing, a BGP peer established a session with BIRD and then sent a KEEPALIVE with length 23 and a 4-byte payload. BIRD treated the malformed message as a normal KEEPALIVE, refreshed the session, and kept the BGP state Established. I am reporting this as a low-severity RFC conformance / input validation issue, not as a high-impact security vulnerability. ### Expected Behavior When BIRD receives a KEEPALIVE message whose BGP message length is not exactly 19 bytes, it should reject the message with: ```text Error Code: Message Header Error Error Subcode: Bad Message Length ``` That is, an oversized KEEPALIVE should not be accepted as a valid KEEPALIVE. ### Actual Behavior BIRD accepts a KEEPALIVE with a 4-byte payload: ```text BGP message type: KEEPALIVE BGP message length: 23 Payload length: 4 bytes Payload bytes: 41 42 43 44 ``` After the malformed KEEPALIVE is sent, `birdc show protocols all` still reports the BGP session as Established: ```text keepalive_len_test BGP --- up ... Established BGP state: Established ``` The BIRD log records the malformed message as a normal KEEPALIVE: ```text keepalive_len_test: Got KEEPALIVE ``` No Header Error / Bad Message Length NOTIFICATION was observed. ### Steps to Reproduce 1. Configure a BGP peer, for example: ```bird router id 127.0.0.1; protocol device { } protocol bgp keepalive_len_test { local 127.0.0.1 port 11879 as 65001; neighbor 127.0.0.2 as 65002; multihop; passive on; ipv4 { import all; export none; }; } ``` 2. Establish the BGP session with normal OPEN and KEEPALIVE messages. 3. Send the malformed KEEPALIVE: ```text ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 00 17 04 41 42 43 44 ``` This is a BGP KEEPALIVE (`type=4`) with total length `0x0017` (23) and a 4-byte body. ### Dynamic Verification This was reproduced against: ```text BIRD version 2.19.0+branch.master.880200b1f94c ``` PoC output: ```text === BIRD oversized KEEPALIVE length PoC === [+] TCP connected: ('127.0.0.2', ...) -> ('127.0.0.1', 11879) [*] Sent BGP OPEN [*] Received pre-test message: type=1 body_len=34 [*] Sent normal KEEPALIVE [*] Received pre-test message: type=4 body_len=0 [*] Sending malformed KEEPALIVE: length=23, payload_len=4 [*] Received post-test message: type=2 body=00000000 ... keepalive_len_test BGP --- up ... Established [+] Socket still writable after malformed KEEPALIVE [!] BIRD accepted an oversized KEEPALIVE and kept the session Established VERDICT: NON_COMPLIANT_REPRODUCED ``` The post-test `type=2 body=00000000` message is an UPDATE carrying an IPv4 End-of-RIB marker. It is not a Header Error NOTIFICATION. ### Source Analysis The generic receive loop checks only that the BGP message length is within the global BGP message bounds: ```c if ((len < BGP_HEADER_LENGTH) || (len > bgp_max_packet_length(conn))) { bgp_error(conn, 1, 2, pkt_start+16, 2); break; } ``` Then `bgp_rx_packet()` dispatches KEEPALIVE messages without passing the packet length to `bgp_rx_keepalive()`: ```c case PKT_KEEPALIVE: return bgp_rx_keepalive(conn); ``` `bgp_rx_keepalive()` therefore cannot reject a KEEPALIVE whose length is greater than 19 bytes: ```c static void bgp_rx_keepalive(struct bgp_conn *conn) { ... bgp_start_timer(conn->hold_timer, conn->hold_time); ... } ``` ### Comparison For comparison, FRR 10.6.1 rejects the same malformed KEEPALIVE with NOTIFICATION `1/2` (`Message Header Error / Bad Message Length`). FRR has an explicit check that KEEPALIVE length must equal 19: ```c type == BGP_MSG_KEEPALIVE && size != BGP_MSG_KEEPALIVE_MIN_SIZE ``` where `BGP_MSG_KEEPALIVE_MIN_SIZE` is `BGP_HEADER_SIZE + 0`. ### Impact The practical impact appears low. A malformed KEEPALIVE is accepted and refreshes the hold timer instead of being rejected as a protocol error. This does not appear to create parser desynchronization, because the extra bytes are consumed as part of the same BGP message. The sender also needs to be an established BGP peer or an on-path attacker capable of injecting/modifying BGP traffic. The issue is best described as an RFC 4271 conformance and malformed input validation bug. ### Suggested Fix Direction Add a KEEPALIVE-specific length check in `bgp_rx_packet()` before dispatching to `bgp_rx_keepalive()`. That avoids changing the `bgp_rx_keepalive()` function signature: ```c static void bgp_rx_packet(struct bgp_conn *conn, byte *pkt, uint len) { byte type = pkt[18]; if ((type == PKT_KEEPALIVE) && (len != BGP_HEADER_LENGTH)) { bgp_error(conn, 1, 2, pkt + 16, 2); return; } ... } ``` The `pkt + 16` data field points to the erroneous Length field, matching the usual Bad Message Length NOTIFICATION data. Best regards.
participants (1)
-
xinzhe liu