Skip to content

Bgp capability common #22254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
donaldsharp wants to merge 2 commits into
base: master
Choose a base branch
from
+52 −84
Open

Bgp capability common #22254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e109eac
bgpd: consolidate capability length validation into a helper
donaldsharp Jun 8, 2026
38ec1b0
bgpd: validate CAPABILITY_CODE_LINK_LOCAL length
donaldsharp Jun 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 48 additions & 47 deletions bgpd/bgp_open.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,54 @@ const size_t cap_modsizes[] = {
[CAPABILITY_CODE_ROLE] = 1,
[CAPABILITY_CODE_SOFT_VERSION] = 1,
[CAPABILITY_CODE_PATHS_LIMIT] = 5,
[CAPABILITY_CODE_LINK_LOCAL] = 1,

@ton31337 ton31337 Jun 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

link-local capability is length of 0, why do we need it here?

};

bool bgp_capability_length_check(struct peer_connection *connection, uint8_t code, uint16_t length)
{
struct peer *peer = connection->peer;

switch (code) {
case CAPABILITY_CODE_MP:
case CAPABILITY_CODE_REFRESH:
case CAPABILITY_CODE_ORF:
case CAPABILITY_CODE_RESTART:
case CAPABILITY_CODE_AS4:
case CAPABILITY_CODE_ADDPATH:
case CAPABILITY_CODE_DYNAMIC:
case CAPABILITY_CODE_ENHE:
case CAPABILITY_CODE_FQDN:
case CAPABILITY_CODE_ENHANCED_RR:
case CAPABILITY_CODE_EXT_MESSAGE:
case CAPABILITY_CODE_ROLE:
case CAPABILITY_CODE_SOFT_VERSION:
case CAPABILITY_CODE_PATHS_LIMIT:
case CAPABILITY_CODE_LLGR:
case CAPABILITY_CODE_LINK_LOCAL:
if (length < cap_minsizes[code]) {
zlog_info("%pBP: %s Capability length error: got %u, expected at least %u",
peer, lookup_msg(capcode_str, code, NULL), length,
(unsigned int)cap_minsizes[code]);
bgp_notify_send(connection, BGP_NOTIFY_OPEN_ERR,
BGP_NOTIFY_OPEN_MALFORMED_ATTR);
return false;
}
if (length && length % cap_modsizes[code] != 0) {
zlog_info("%pBP: %s Capability length error: got %u, expected a multiple of %u",
peer, lookup_msg(capcode_str, code, NULL), length,
(unsigned int)cap_modsizes[code]);
bgp_notify_send(connection, BGP_NOTIFY_OPEN_ERR,
BGP_NOTIFY_OPEN_MALFORMED_ATTR);
return false;
}
break;
default:
break;
}
Comment on lines +99 to +135

@greptile-apps greptile-apps Bot Jun 8, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 CAPABILITY_CODE_LLGR missing from shared length-check

CAPABILITY_CODE_LLGR has entries in both cap_minsizes and cap_modsizes (lines 64 and 88 respectively), but it is absent from the switch in bgp_capability_length_check. This was pre-existing in both original switch blocks, but now that those blocks are consolidated into a single authoritative function, it becomes more visible: LLGR capability lengths are never validated. Any peer sending a malformed LLGR capability can bypass the sanity check entirely.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: bgpd/bgp_open.c
Line: 99-134

Comment:
**`CAPABILITY_CODE_LLGR` missing from shared length-check**

`CAPABILITY_CODE_LLGR` has entries in both `cap_minsizes` and `cap_modsizes` (lines 64 and 88 respectively), but it is absent from the switch in `bgp_capability_length_check`. This was pre-existing in both original switch blocks, but now that those blocks are consolidated into a single authoritative function, it becomes more visible: LLGR capability lengths are never validated. Any peer sending a malformed LLGR capability can bypass the sanity check entirely.

How can I resolve this? If you propose a fix, please make it concise.


return true;
}

/* BGP-4 Multiprotocol Extensions lead us to the complex world. We can
negotiate remote peer supports extensions or not. But if
remote-peer doesn't supports negotiation process itself. We would
Expand Down Expand Up @@ -1116,53 +1162,8 @@ static int bgp_capability_parse(struct peer_connection *connection, size_t lengt
caphdr.code, caphdr.length);

/* Length sanity check, type-specific, for known capabilities */
switch (caphdr.code) {
case CAPABILITY_CODE_MP:
case CAPABILITY_CODE_REFRESH:
case CAPABILITY_CODE_ORF:
case CAPABILITY_CODE_RESTART:
case CAPABILITY_CODE_AS4:
case CAPABILITY_CODE_ADDPATH:
case CAPABILITY_CODE_DYNAMIC:
case CAPABILITY_CODE_ENHE:
case CAPABILITY_CODE_FQDN:
case CAPABILITY_CODE_ENHANCED_RR:
case CAPABILITY_CODE_EXT_MESSAGE:
case CAPABILITY_CODE_ROLE:
case CAPABILITY_CODE_SOFT_VERSION:
case CAPABILITY_CODE_PATHS_LIMIT:
case CAPABILITY_CODE_LLGR:
/* Check length. */
if (caphdr.length < cap_minsizes[caphdr.code]) {
zlog_info(
"%s %s Capability length error: got %u, expected at least %u",
peer->host,
lookup_msg(capcode_str, caphdr.code,
NULL),
caphdr.length,
(unsigned)cap_minsizes[caphdr.code]);
bgp_notify_send(connection, BGP_NOTIFY_OPEN_ERR,
BGP_NOTIFY_OPEN_MALFORMED_ATTR);
return -1;
}
if (caphdr.length
&& caphdr.length % cap_modsizes[caphdr.code] != 0) {
zlog_info(
"%s %s Capability length error: got %u, expected a multiple of %u",
peer->host,
lookup_msg(capcode_str, caphdr.code,
NULL),
caphdr.length,
(unsigned)cap_modsizes[caphdr.code]);
bgp_notify_send(connection, BGP_NOTIFY_OPEN_ERR,
BGP_NOTIFY_OPEN_MALFORMED_ATTR);
return -1;
}
break;
/* we deliberately ignore unknown codes, see below */
default:
break;
}
if (!bgp_capability_length_check(connection, caphdr.code, caphdr.length))
return -1;

switch (caphdr.code) {
case CAPABILITY_CODE_MP: {
Expand Down
2 changes: 2 additions & 0 deletions bgpd/bgp_open.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ extern uint16_t bgp_open_capability(struct stream *s, struct peer_connection *co
extern void bgp_capability_vty_out(struct vty *vty, struct peer *peer,
bool use_json, json_object *json_neigh);
extern as_t peek_for_as4_capability(struct peer_connection *connection, uint16_t length);
extern bool bgp_capability_length_check(struct peer_connection *connection, uint8_t code,
uint16_t length);
extern const struct message capcode_str[];
extern const struct message orf_type_str[];
extern const struct message orf_mode_str[];
Expand Down
39 changes: 2 additions & 37 deletions bgpd/bgp_packet.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3827,43 +3827,8 @@ static int bgp_capability_msg_parse(struct peer_connection *connection, uint8_t
capability = lookup_msg(capcode_str, hdr->code, "Unknown");

/* Length sanity check, type-specific, for known capabilities */
switch (hdr->code) {
case CAPABILITY_CODE_MP:
case CAPABILITY_CODE_REFRESH:
case CAPABILITY_CODE_ORF:
case CAPABILITY_CODE_RESTART:
case CAPABILITY_CODE_AS4:
case CAPABILITY_CODE_ADDPATH:
case CAPABILITY_CODE_DYNAMIC:
case CAPABILITY_CODE_ENHE:
case CAPABILITY_CODE_FQDN:
case CAPABILITY_CODE_ENHANCED_RR:
case CAPABILITY_CODE_EXT_MESSAGE:
case CAPABILITY_CODE_ROLE:
case CAPABILITY_CODE_SOFT_VERSION:
case CAPABILITY_CODE_PATHS_LIMIT:
case CAPABILITY_CODE_LLGR:
if (hdr->length < cap_minsizes[hdr->code]) {
zlog_info("%pBP: %s Capability length error: got %u, expected at least %u",
peer, capability, hdr->length,
(unsigned int)cap_minsizes[hdr->code]);
bgp_notify_send(connection, BGP_NOTIFY_OPEN_ERR,
BGP_NOTIFY_OPEN_MALFORMED_ATTR);
goto done;
}
if (hdr->length &&
hdr->length % cap_modsizes[hdr->code] != 0) {
zlog_info("%pBP %s Capability length error: got %u, expected a multiple of %u",
peer, capability, hdr->length,
(unsigned int)cap_modsizes[hdr->code]);
bgp_notify_send(connection, BGP_NOTIFY_OPEN_ERR,
BGP_NOTIFY_OPEN_MALFORMED_ATTR);
goto done;
}
break;
default:
break;
}
if (!bgp_capability_length_check(connection, hdr->code, hdr->length))
goto done;

switch (hdr->code) {
case CAPABILITY_CODE_SOFT_VERSION:
Expand Down
Loading