From 30adbd4e4fb66e7e48e2bcb8f62f5c68b1be45b7 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 2 May 2018 14:03:39 -0400 Subject: [PATCH 1/2] bgpd: Handle multiple PREFIX_SID's at a time. Handle multiple PREFIX_SID's at the same time. The draft clearly states that multiple should be handled and we have a actual pcap file that clearly has multiple PREFIX_SID's at the same time. Fixes: #2153 Signed-off-by: Donald Sharp --- bgpd/bgp_attr.c | 85 +++++++++++++++++++++++++++++++++++-------------- bgpd/bgp_attr.h | 3 ++ 2 files changed, 64 insertions(+), 24 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index b6e9ee3cb2..276a7054e3 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -2021,36 +2021,32 @@ static int bgp_attr_encap(uint8_t type, struct peer *peer, /* IN */ return 0; } -/* Prefix SID attribute - * draft-ietf-idr-bgp-prefix-sid-05 +/* + * Read an individual SID value returning how much data we have read + * Returns 0 if there was an error that needs to be passed up the stack */ -static bgp_attr_parse_ret_t -bgp_attr_prefix_sid(struct bgp_attr_parser_args *args, - struct bgp_nlri *mp_update) +static bgp_attr_parse_ret_t bgp_attr_psid_sub(int32_t type, + int32_t length, + struct bgp_attr_parser_args *args, + struct bgp_nlri *mp_update) { struct peer *const peer = args->peer; struct attr *const attr = args->attr; - int type; - int length; uint32_t label_index; struct in6_addr ipv6_sid; uint32_t srgb_base; uint32_t srgb_range; int srgb_count; - attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID); - - type = stream_getc(peer->curr); - length = stream_getw(peer->curr); - if (type == BGP_PREFIX_SID_LABEL_INDEX) { if (length != BGP_PREFIX_SID_LABEL_INDEX_LENGTH) { zlog_err( - "Prefix SID label index length is %d instead of %d", - length, BGP_PREFIX_SID_LABEL_INDEX_LENGTH); - return bgp_attr_malformed( - args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - args->total); + "Prefix SID label index length is %d instead of %d", + length, + BGP_PREFIX_SID_LABEL_INDEX_LENGTH); + return bgp_attr_malformed(args, + BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, + args->total); } /* Ignore flags and reserved */ @@ -2060,9 +2056,8 @@ bgp_attr_prefix_sid(struct bgp_attr_parser_args *args, /* Fetch the label index and see if it is valid. */ label_index = stream_getl(peer->curr); if (label_index == BGP_INVALID_LABEL_INDEX) - return bgp_attr_malformed( - args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR, - args->total); + return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR, + args->total); /* Store label index; subsequently, we'll check on * address-family */ @@ -2083,9 +2078,9 @@ bgp_attr_prefix_sid(struct bgp_attr_parser_args *args, if (length != BGP_PREFIX_SID_IPV6_LENGTH) { zlog_err("Prefix SID IPv6 length is %d instead of %d", length, BGP_PREFIX_SID_IPV6_LENGTH); - return bgp_attr_malformed( - args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - args->total); + return bgp_attr_malformed(args, + BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, + args->total); } /* Ignore reserved */ @@ -2122,6 +2117,47 @@ bgp_attr_prefix_sid(struct bgp_attr_parser_args *args, return BGP_ATTR_PARSE_PROCEED; } +/* Prefix SID attribute + * draft-ietf-idr-bgp-prefix-sid-05 + */ +bgp_attr_parse_ret_t +bgp_attr_prefix_sid(int32_t tlength, struct bgp_attr_parser_args *args, + struct bgp_nlri *mp_update) +{ + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + bgp_attr_parse_ret_t ret; + + attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_PREFIX_SID); + + while (tlength) { + int32_t type, length; + + type = stream_getc(peer->curr); + length = stream_getw(peer->curr); + + ret = bgp_attr_psid_sub(type, length, args, mp_update); + + if (ret != BGP_ATTR_PARSE_PROCEED) + return ret; + /* + * Subtract length + the T and the L + * since length is the Vector portion + */ + tlength -= length + 3; + + if (tlength < 0) { + zlog_err("Prefix SID internal length %d causes us to read beyond the total Prefix SID length", + length); + return bgp_attr_malformed(args, + BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, + args->total); + } + } + + return BGP_ATTR_PARSE_PROCEED; +} + /* PMSI tunnel attribute (RFC 6514) * Basic validation checks done here. */ @@ -2498,7 +2534,8 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, startp); break; case BGP_ATTR_PREFIX_SID: - ret = bgp_attr_prefix_sid(&attr_args, mp_update); + ret = bgp_attr_prefix_sid(length, + &attr_args, mp_update); break; case BGP_ATTR_PMSI_TUNNEL: ret = bgp_attr_pmsi_tunnel(&attr_args); diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index d59eae5f14..f17c2a68e4 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -308,6 +308,9 @@ extern int bgp_mp_reach_parse(struct bgp_attr_parser_args *args, struct bgp_nlri *); extern int bgp_mp_unreach_parse(struct bgp_attr_parser_args *args, struct bgp_nlri *); +extern bgp_attr_parse_ret_t +bgp_attr_prefix_sid(int32_t tlength, struct bgp_attr_parser_args *args, + struct bgp_nlri *mp_update); extern struct bgp_attr_encap_subtlv * encap_tlv_dup(struct bgp_attr_encap_subtlv *orig); From 4258abc56c724c57185184f5c55bb7a22ab100e5 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 2 May 2018 18:46:18 -0400 Subject: [PATCH 2/2] tests: Add a prefix-sid test Signed-off-by: Donald Sharp --- tests/bgpd/test_mp_attr.c | 38 +++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/tests/bgpd/test_mp_attr.c b/tests/bgpd/test_mp_attr.c index 9e0cb1f5ca..8db1cb2ca1 100644 --- a/tests/bgpd/test_mp_attr.c +++ b/tests/bgpd/test_mp_attr.c @@ -945,6 +945,24 @@ static struct test_segment mp_unreach_segments[] = { }, {NULL, NULL, {0}, 0, 0}}; +static struct test_segment mp_prefix_sid[] = { + { + "PREFIX-SID", + "PREFIX-SID Test 1", + { + 0x01, 0x00, 0x07, + 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x02, + 0x03, 0x00, 0x08, 0x00, + 0x00, 0x0a, 0x1b, 0xfe, + 0x00, 0x00, 0x0a + }, + .len = 21, + .parses = SHOULD_PARSE, + }, + {NULL, NULL, { 0 }, 0, 0}, +}; + /* nlri_parse indicates 0 on successful parse, and -1 otherwise. * attr_parse indicates BGP_ATTR_PARSE_PROCEED/0 on success, * and BGP_ATTR_PARSE_ERROR/-1 or lower negative ret on err. @@ -1000,10 +1018,20 @@ static void parse_test(struct peer *peer, struct test_segment *t, int type) printf("%s: %s\n", t->name, t->desc); - if (type == BGP_ATTR_MP_REACH_NLRI) + switch (type) { + case BGP_ATTR_MP_REACH_NLRI: parse_ret = bgp_mp_reach_parse(&attr_args, &nlri); - else + break; + case BGP_ATTR_MP_UNREACH_NLRI: parse_ret = bgp_mp_unreach_parse(&attr_args, &nlri); + break; + case BGP_ATTR_PREFIX_SID: + parse_ret = bgp_attr_prefix_sid(t->len, &attr_args, &nlri); + break; + default: + printf("unknown type"); + return; + } if (!parse_ret) { iana_afi_t pkt_afi; iana_safi_t pkt_safi; @@ -1022,7 +1050,7 @@ static void parse_test(struct peer *peer, struct test_segment *t, int type) if (!parse_ret) { if (type == BGP_ATTR_MP_REACH_NLRI) nlri_ret = bgp_nlri_parse(peer, &attr, &nlri, 0); - else + else if (type == BGP_ATTR_MP_UNREACH_NLRI) nlri_ret = bgp_nlri_parse(peer, &attr, &nlri, 1); } handle_result(peer, t, parse_ret, nlri_ret); @@ -1085,6 +1113,10 @@ int main(void) parse_test(peer, &mp_unreach_segments[i++], BGP_ATTR_MP_UNREACH_NLRI); + i = 0; + while (mp_prefix_sid[i].name) + parse_test(peer, &mp_prefix_sid[i++], + BGP_ATTR_PREFIX_SID); printf("failures: %d\n", failed); return failed; }