From 0e62ddbd7e0c088e925b550f7cfeb6b5b327e016 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 6 Oct 2023 10:21:56 +0300 Subject: [PATCH 1/3] bgpd: BGP Link-State attribute should be treated as withdraw According to https://datatracker.ietf.org/doc/html/rfc7606 BGP-LS Attribute does have error handling consistent with Section 8 and thus are not further discussed herein. Section 8: The "treat-as-withdraw" approach is generally preferred and the "session reset" approach is discouraged. Signed-off-by: Donatas Abraitis --- bgpd/bgp_attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 808b98e41998..4ebce4cfd8c2 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1521,8 +1521,8 @@ bgp_attr_malformed(struct bgp_attr_parser_args *args, uint8_t subcode, case BGP_ATTR_PMSI_TUNNEL: case BGP_ATTR_ENCAP: case BGP_ATTR_OTC: - return BGP_ATTR_PARSE_WITHDRAW; case BGP_ATTR_LINK_STATE: + return BGP_ATTR_PARSE_WITHDRAW; case BGP_ATTR_MP_REACH_NLRI: case BGP_ATTR_MP_UNREACH_NLRI: bgp_notify_send_with_data(peer->connection, From 4b5ba17109b21ba7c19d0eb9b23ddd991fd290be Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 6 Oct 2023 10:32:29 +0300 Subject: [PATCH 2/3] bgpd: Check if we have enough stream data when handling BGP-LS attribute Signed-off-by: Donatas Abraitis --- bgpd/bgp_attr.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 4ebce4cfd8c2..de82a3c52894 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -3417,6 +3417,14 @@ bgp_attr_linkstate(struct bgp_attr_parser_args *args) if (STREAM_READABLE(s) == 0) return BGP_ATTR_PARSE_PROCEED; + /* Length check. */ + if (STREAM_READABLE(s) < length) { + flog_err(EC_BGP_ATTR_LEN, + "BGP-LS attribute length is too big [%u]", length); + return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, + args->total); + } + attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_LINK_STATE); bgp_attr_ls = XCALLOC(MTYPE_BGP_ATTR_LS, sizeof(struct bgp_attr_ls)); From a2cb91e439d4cf58b3bcc5884add75f9b8869ce6 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Fri, 6 Oct 2023 10:39:01 +0300 Subject: [PATCH 3/3] bgpd: BGP-LS attribute ca be much bigger than only 1 byte Increase to 2 bytes and also check if we don't go beyond the maximum packet size. When a BGP-LS Propagator finds that it is exceeding the maximum BGP message size due to the addition or update of some other BGP Attribute , it MUST consider the BGP-LS Attribute to be malformed, apply the "Attribute Discard" error-handling approach. Signed-off-by: Donatas Abraitis --- bgpd/bgp_attr.c | 17 ++++++++++++++--- bgpd/bgp_attr.h | 2 +- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index de82a3c52894..1ed111815059 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -4990,10 +4990,21 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer, } /* BGP Link-State */ - if (attr->link_state) { - stream_putc(s, BGP_ATTR_FLAG_OPTIONAL); + if (attr->link_state && + attr->link_state->length < peer->max_packet_size) { + uint8_t flags = BGP_ATTR_FLAG_OPTIONAL; + + if (attr->link_state->length > 255) + SET_FLAG(flags, BGP_ATTR_FLAG_EXTLEN); + + stream_putc(s, flags); stream_putc(s, BGP_ATTR_LINK_STATE); - stream_putc(s, attr->link_state->length); + + if (CHECK_FLAG(flags, BGP_ATTR_FLAG_EXTLEN)) + stream_putw(s, attr->link_state->length); + else + stream_putc(s, attr->link_state->length); + stream_put(s, attr->link_state->data, attr->link_state->length); } diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index e637b0efbfd3..817895fdaaae 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -139,7 +139,7 @@ struct bgp_attr_srv6_l3vpn { struct bgp_attr_ls { unsigned long refcnt; - uint8_t length; + uint16_t length; void *data; };