Skip to content
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

Jump to bottom

bgpd: Fix error handling when receiving BGP Prefix SID attribute (backport #15628) #15658

Merged
merged 2 commits into from
Apr 3, 2024
Merged

bgpd: Fix error handling when receiving BGP Prefix SID attribute (backport #15628) #15658

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0486f4f
bgpd: Fix error handling when receiving BGP Prefix SID attribute
ton31337 Mar 27, 2024
1867855
bgpd: Prevent from one more CVE triggering this place
ton31337 Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 25 additions & 13 deletions bgpd/bgp_attr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1375,6 +1375,15 @@ bgp_attr_malformed(struct bgp_attr_parser_args *args, uint8_t subcode,
(args->startp - STREAM_DATA(BGP_INPUT(peer)))
+ args->total);

/* Partial optional attributes that are malformed should not cause
* the whole session to be reset. Instead treat it as a withdrawal
* of the routes, if possible.
*/
if (CHECK_FLAG(flags, BGP_ATTR_FLAG_TRANS) &&
CHECK_FLAG(flags, BGP_ATTR_FLAG_OPTIONAL) &&
CHECK_FLAG(flags, BGP_ATTR_FLAG_PARTIAL))
return BGP_ATTR_PARSE_WITHDRAW;

switch (args->type) {
/* where an attribute is relatively inconsequential, e.g. it does not
* affect route selection, and can be safely ignored, then any such
Expand All @@ -1384,6 +1393,7 @@ bgp_attr_malformed(struct bgp_attr_parser_args *args, uint8_t subcode,
case BGP_ATTR_AS4_AGGREGATOR:
case BGP_ATTR_AGGREGATOR:
case BGP_ATTR_ATOMIC_AGGREGATE:
case BGP_ATTR_PREFIX_SID:
return BGP_ATTR_PARSE_PROCEED;

/* Core attributes, particularly ones which may influence route
Expand All @@ -1410,19 +1420,21 @@ bgp_attr_malformed(struct bgp_attr_parser_args *args, uint8_t subcode,
bgp_notify_send_with_data(peer, BGP_NOTIFY_UPDATE_ERR, subcode,
notify_datap, length);
return BGP_ATTR_PARSE_ERROR;
default:
/* Unknown attributes, that are handled by this function
* should be treated as withdraw, to prevent one more CVE
* from being introduced.
* RFC 7606 says:
* The "treat-as-withdraw" approach is generally preferred
* and the "session reset" approach is discouraged.
*/
flog_err(EC_BGP_ATTR_FLAG,
"%s(%u) attribute received, while it is not known how to handle it, treating as withdraw",
lookup_msg(attr_str, args->type, NULL), args->type);
break;
}

/* Partial optional attributes that are malformed should not cause
* the whole session to be reset. Instead treat it as a withdrawal
* of the routes, if possible.
*/
if (CHECK_FLAG(flags, BGP_ATTR_FLAG_TRANS)
&& CHECK_FLAG(flags, BGP_ATTR_FLAG_OPTIONAL)
&& CHECK_FLAG(flags, BGP_ATTR_FLAG_PARTIAL))
return BGP_ATTR_PARSE_WITHDRAW;

/* default to reset */
return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
return BGP_ATTR_PARSE_WITHDRAW;
}

/* Find out what is wrong with the path attribute flag bits and log the error.
Expand Down Expand Up @@ -3108,8 +3120,6 @@ enum bgp_attr_parse_ret bgp_attr_prefix_sid(struct bgp_attr_parser_args *args)
struct attr *const attr = args->attr;
enum bgp_attr_parse_ret ret;

attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID);

uint8_t type;
uint16_t length;
size_t headersz = sizeof(type) + sizeof(length);
Expand Down Expand Up @@ -3159,6 +3169,8 @@ enum bgp_attr_parse_ret bgp_attr_prefix_sid(struct bgp_attr_parser_args *args)
}
}

SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID));

return BGP_ATTR_PARSE_PROCEED;
}

Expand Down