From 835315bfb49bff2b2fb354f2075c6d6693c2a151 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Wed, 18 Jan 2012 12:28:30 +0000 Subject: bgpd: Move up flag-check calls, parcel up attr-parser args, and other cleanups * bgp_attr.h: (struct bgp_attr_parser_args) Attribute parsing context, containing common arguments. * bgp_attr.c: (general) Move the bgp_attr_flag_invalid flag-check calls up, out of each individual attr parser function, to be done once in attr_parse. Similarly move the calculation of the 'total' attribute length field up to attr_parse. Bundle together common arguments to attr-parsing functions and helpers into (struct bgp_attr_parser_args), so it can be passed by reference down the stack & also de-clutter the argument lists & make it easier to add/modify the context for attr-parsing - add local const aliases to avoid modifying body of code too much. This also should help avoid cut & paste errors, where calls to helpers with hard-coded attribute types are pasted to other functions but the code isn't changed. (bgp_attr_flags_diagnose) as above. (bgp_attr_flag_invalid) as above. (bgp_attr_{origin,aspath,as4_path,nexthop,med,local_pref,atomic}) as above. (bgp_attr_{aggregator,as4_aggregator,community,originator_id}) as above (bgp_attr_{cluster_list,ext_communities},bgp_mp_{un,}reach_parse) as above (bgp_attr_unknown) as above. (bgp_attr_malformed) as above. Also, startp and length have to be special-cased, because whether or not to send attribute data depends on the particular error - a separate length argument, distinct from args->length, indicates whether or not the attribute data should be sent in the NOTIFY. (bgp_attr_aspath_check) Call to bgp_attr_malformed is wrong here, there is no attribute parsing context - e.g. the 'flag' argument is unlikely to be right, remove it. Explicitly handle the error instead. (bgp_attr_munge_as4_attrs) Flag argument is pointless. As the comment notes, the check here is pointless as AS_PATH presence already checked elsewhere. (bgp_attr_parse) Do bgp_attr_flag_invalid call here. Use (struct bgp_attr_parser_args) for args to attr parser functions. Remove out-of-context 'flag' argument to as4 checking functions. --- bgpd/bgp_attr.c | 467 +++++++++++++++++++++++--------------------------------- bgpd/bgp_attr.h | 19 ++- 2 files changed, 208 insertions(+), 278 deletions(-) (limited to 'bgpd') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index d8ca831a..b02cfee3 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -710,9 +710,17 @@ bgp_attr_flush (struct attr *attr) * introduced by the sending neighbour. */ static bgp_attr_parse_ret_t -bgp_attr_malformed (struct peer *peer, u_char type, u_char flag, - u_char subcode, u_char *startp, bgp_size_t length) -{ +bgp_attr_malformed (struct bgp_attr_parser_args *args, u_char subcode, + bgp_size_t length) +{ + struct peer *const peer = args->peer; + const u_int8_t flags = args->flags; + /* startp and length must be special-cased, as whether or not to + * send the attribute data with the NOTIFY depends on the error, + * the caller therefore signals this with the seperate length argument + */ + u_char *startp = (length > 0 ? args->startp : NULL); + /* Only relax error handling for eBGP peers */ if (peer_sort (peer) != BGP_PEER_EBGP) { @@ -722,7 +730,7 @@ bgp_attr_malformed (struct peer *peer, u_char type, u_char flag, } - switch (type) { + switch (args->type) { /* where an optional attribute is inconsequential, e.g. it does not affect * route selection, and can be safely ignored then any such attributes * which are malformed should just be ignored and the route processed as @@ -756,9 +764,9 @@ bgp_attr_malformed (struct peer *peer, u_char type, u_char flag, * the whole session to be reset. Instead treat it as a withdrawal * of the routes, if possible. */ - if (CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS) - && CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL) - && CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL)) + 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 */ @@ -771,16 +779,14 @@ bgp_attr_malformed (struct peer *peer, u_char type, u_char flag, being diagnosed is defined by RFC as either a "well-known" or an "optional, non-transitive" attribute. */ static void -bgp_attr_flags_diagnose -( - struct peer * peer, - const u_int8_t attr_code, - u_int8_t desired_flags, /* how RFC says it must be */ - u_int8_t real_flags /* on wire */ +bgp_attr_flags_diagnose (struct bgp_attr_parser_args *args, + u_int8_t desired_flags /* how RFC says it must be */ ) { u_char seen = 0, i; - + u_char real_flags = args->flags; + const u_int8_t attr_code = args->type; + desired_flags &= ~BGP_ATTR_FLAG_EXTLEN; real_flags &= ~BGP_ATTR_FLAG_EXTLEN; for (i = 0; i <= 2; i++) /* O,T,P, but not E */ @@ -790,7 +796,7 @@ bgp_attr_flags_diagnose CHECK_FLAG (real_flags, attr_flag_str[i].key) ) { - zlog (peer->log, LOG_ERR, "%s attribute must%s be flagged as \"%s\"", + zlog (args->peer->log, LOG_ERR, "%s attribute must%s be flagged as \"%s\"", LOOKUP (attr_str, attr_code), CHECK_FLAG (desired_flags, attr_flag_str[i].key) ? "" : " not", attr_flag_str[i].str); @@ -823,9 +829,12 @@ static const size_t attr_flags_values_max = sizeof (attr_flags_values) / sizeof (attr_flags_values[0]); static int -bgp_attr_flag_invalid (struct peer *peer, u_int8_t attr_code, u_int8_t flags) +bgp_attr_flag_invalid (struct bgp_attr_parser_args *args) { u_int8_t mask = BGP_ATTR_FLAG_EXTLEN; + const u_int8_t flags = args->flags; + const u_int8_t attr_code = args->type; + struct peer *const peer = args->peer; /* there may be attributes we don't know about */ if (attr_code > attr_flags_values_max) @@ -880,31 +889,18 @@ bgp_attr_flag_invalid (struct peer *peer, u_int8_t attr_code, u_int8_t flags) == attr_flags_values[attr_code]) return 0; - bgp_attr_flags_diagnose (peer, attr_code, attr_flags_values[attr_code], - flags); + bgp_attr_flags_diagnose (args, attr_flags_values[attr_code]); return 1; } /* Get origin attribute of the update message. */ static bgp_attr_parse_ret_t -bgp_attr_origin (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, u_char *startp) +bgp_attr_origin (struct bgp_attr_parser_args *args) { - bgp_size_t total; - - /* total is entire attribute length include Attribute Flags (1), - Attribute Type code (1) and Attribute length (1 or 2). */ - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - - /* If any recognized attribute has Attribute Flags that conflict - with the Attribute Type Code, then the Error Subcode is set to - Attribute Flags Error. The Data field contains the erroneous - attribute (type, length and value). */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_ORIGIN, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); - + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + const bgp_size_t length = args->length; + /* If any recognized attribute has Attribute Length that conflicts with the expected length (based on the attribute type code), then the Error Subcode is set to Attribute Length Error. The Data @@ -914,9 +910,9 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length, { zlog (peer->log, LOG_ERR, "Origin attribute length is not one %d", length); - return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag, + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - startp, total); + args->total); } /* Fetch origin attribute. */ @@ -931,9 +927,9 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length, { zlog (peer->log, LOG_ERR, "Origin attribute value is invalid %d", attr->origin); - return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag, + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_INVAL_ORIGIN, - startp, total); + args->total); } /* Set oring attribute flag. */ @@ -945,17 +941,12 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length, /* Parse AS path information. This function is wrapper of aspath_parse. */ static int -bgp_attr_aspath (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, u_char *startp) +bgp_attr_aspath (struct bgp_attr_parser_args *args) { - bgp_size_t total; - - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); + struct attr *const attr = args->attr; + struct peer *const peer = args->peer; + const bgp_size_t length = args->length; - if (bgp_attr_flag_invalid (peer, BGP_ATTR_AS_PATH, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); /* * peer with AS4 => will get 4Byte ASnums * otherwise, will get 16 Bit @@ -969,9 +960,7 @@ bgp_attr_aspath (struct peer *peer, bgp_size_t length, zlog (peer->log, LOG_ERR, "Malformed AS path from %s, length is %d", peer->host, length); - return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag, - BGP_NOTIFY_UPDATE_MAL_AS_PATH, - NULL, 0); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_MAL_AS_PATH, 0); } /* Set aspath attribute flag. */ @@ -981,7 +970,7 @@ bgp_attr_aspath (struct peer *peer, bgp_size_t length, } static bgp_attr_parse_ret_t -bgp_attr_aspath_check (struct peer *peer, struct attr *attr, u_char flag) +bgp_attr_aspath_check (struct peer *const peer, struct attr *const attr) { /* These checks were part of bgp_attr_aspath, but with * as4 we should to check aspath things when @@ -1000,9 +989,9 @@ bgp_attr_aspath_check (struct peer *peer, struct attr *attr, u_char flag) (peer_sort (peer) == BGP_PEER_EBGP && aspath_confed_check (attr->aspath))) { zlog (peer->log, LOG_ERR, "Malformed AS path from %s", peer->host); - return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag, - BGP_NOTIFY_UPDATE_MAL_AS_PATH, - NULL, 0); + bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_MAL_AS_PATH); + return BGP_ATTR_PARSE_ERROR; } /* First AS check for EBGP. */ @@ -1013,9 +1002,9 @@ bgp_attr_aspath_check (struct peer *peer, struct attr *attr, u_char flag) { zlog (peer->log, LOG_ERR, "%s incorrect first AS (must be %u)", peer->host, peer->as); - return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag, - BGP_NOTIFY_UPDATE_MAL_AS_PATH, - NULL, 0); + bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_MAL_AS_PATH); + return BGP_ATTR_PARSE_ERROR; } } @@ -1035,19 +1024,11 @@ bgp_attr_aspath_check (struct peer *peer, struct attr *attr, u_char flag) /* Parse AS4 path information. This function is another wrapper of aspath_parse. */ static int -bgp_attr_as4_path (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, u_char *startp, - struct aspath **as4_path) +bgp_attr_as4_path (struct bgp_attr_parser_args *args, struct aspath **as4_path) { - bgp_size_t total; - - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - - /* Flag check. */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_AS4_PATH, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_AS4_PATH, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + const bgp_size_t length = args->length; *as4_path = aspath_parse (peer->ibuf, length, 1); @@ -1057,9 +1038,9 @@ bgp_attr_as4_path (struct peer *peer, bgp_size_t length, zlog (peer->log, LOG_ERR, "Malformed AS4 path from %s, length is %d", peer->host, length); - return bgp_attr_malformed (peer, BGP_ATTR_AS4_PATH, flag, + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_MAL_AS_PATH, - NULL, 0); + 0); } /* Set aspath attribute flag. */ @@ -1071,29 +1052,23 @@ bgp_attr_as4_path (struct peer *peer, bgp_size_t length, /* Nexthop attribute. */ static bgp_attr_parse_ret_t -bgp_attr_nexthop (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, u_char *startp) +bgp_attr_nexthop (struct bgp_attr_parser_args *args) { - bgp_size_t total; + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + const bgp_size_t length = args->length; + in_addr_t nexthop_h, nexthop_n; - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - - /* Flags check. */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_NEXT_HOP, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); - /* Check nexthop attribute length. */ if (length != 4) { zlog (peer->log, LOG_ERR, "Nexthop attribute length isn't four [%d]", length); - return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag, + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - startp, total); + args->total); } /* According to section 6.3 of RFC4271, syntactically incorrect NEXT_HOP @@ -1108,9 +1083,9 @@ bgp_attr_nexthop (struct peer *peer, bgp_size_t length, char buf[INET_ADDRSTRLEN]; inet_ntop (AF_INET, &nexthop_h, buf, INET_ADDRSTRLEN); zlog (peer->log, LOG_ERR, "Martian nexthop %s", buf); - return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag, + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP, - startp, total); + args->total); } attr->nexthop.s_addr = nexthop_n; @@ -1121,28 +1096,21 @@ bgp_attr_nexthop (struct peer *peer, bgp_size_t length, /* MED atrribute. */ static bgp_attr_parse_ret_t -bgp_attr_med (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, u_char *startp) +bgp_attr_med (struct bgp_attr_parser_args *args) { - bgp_size_t total; - - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - - /* Flag checks. */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_MULTI_EXIT_DISC, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_MULTI_EXIT_DISC, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); - + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + const bgp_size_t length = args->length; + /* Length check. */ if (length != 4) { zlog (peer->log, LOG_ERR, "MED attribute length isn't four [%d]", length); - return bgp_attr_malformed (peer, BGP_ATTR_MULTI_EXIT_DISC, flag, + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - startp, total); + args->total); } attr->med = stream_getl (peer->ibuf); @@ -1154,25 +1122,20 @@ bgp_attr_med (struct peer *peer, bgp_size_t length, /* Local preference attribute. */ static bgp_attr_parse_ret_t -bgp_attr_local_pref (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, u_char *startp) +bgp_attr_local_pref (struct bgp_attr_parser_args *args) { - bgp_size_t total; - - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - /* Flag checks. */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_LOCAL_PREF, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_LOCAL_PREF, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + const bgp_size_t length = args->length; /* Length check. */ if (length != 4) { - zlog (peer->log, LOG_ERR, "LOCAL_PREF attribute length isn't 4 [%u]", length); - return bgp_attr_malformed (peer, BGP_ATTR_LOCAL_PREF, flag, + zlog (peer->log, LOG_ERR, "LOCAL_PREF attribute length isn't 4 [%u]", + length); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - startp, total); + args->total); } /* If it is contained in an UPDATE message that is received from an @@ -1194,25 +1157,20 @@ bgp_attr_local_pref (struct peer *peer, bgp_size_t length, /* Atomic aggregate. */ static int -bgp_attr_atomic (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, u_char *startp) +bgp_attr_atomic (struct bgp_attr_parser_args *args) { - bgp_size_t total; - - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - /* Flag checks. */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_ATOMIC_AGGREGATE, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_ATOMIC_AGGREGATE, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + const bgp_size_t length = args->length; /* Length check. */ if (length != 0) { - zlog (peer->log, LOG_ERR, "ATOMIC_AGGREGATE attribute length isn't 0 [%u]", length); - return bgp_attr_malformed (peer, BGP_ATTR_ATOMIC_AGGREGATE, flag, + zlog (peer->log, LOG_ERR, "ATOMIC_AGGREGATE attribute length isn't 0 [%u]", + length); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - startp, total); + args->total); } /* Set atomic aggregate flag. */ @@ -1223,19 +1181,14 @@ bgp_attr_atomic (struct peer *peer, bgp_size_t length, /* Aggregator attribute */ static int -bgp_attr_aggregator (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, u_char *startp) +bgp_attr_aggregator (struct bgp_attr_parser_args *args) { + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + const bgp_size_t length = args->length; + int wantedlen = 6; struct attr_extra *attre = bgp_attr_extra_get (attr); - bgp_size_t total; - - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - /* Flags check. */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_AGGREGATOR, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_AGGREGATOR, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); /* peer with AS4 will send 4 Byte AS, peer without will send 2 Byte */ if (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV)) @@ -1243,10 +1196,11 @@ bgp_attr_aggregator (struct peer *peer, bgp_size_t length, if (length != wantedlen) { - zlog (peer->log, LOG_ERR, "AGGREGATOR attribute length isn't %u [%u]", wantedlen, length); - return bgp_attr_malformed (peer, BGP_ATTR_AGGREGATOR, flag, + zlog (peer->log, LOG_ERR, "AGGREGATOR attribute length isn't %u [%u]", + wantedlen, length); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - startp, total); + args->total); } if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV ) ) @@ -1263,27 +1217,22 @@ bgp_attr_aggregator (struct peer *peer, bgp_size_t length, /* New Aggregator attribute */ static bgp_attr_parse_ret_t -bgp_attr_as4_aggregator (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, - as_t *as4_aggregator_as, - struct in_addr *as4_aggregator_addr, - u_char *startp) +bgp_attr_as4_aggregator (struct bgp_attr_parser_args *args, + as_t *as4_aggregator_as, + struct in_addr *as4_aggregator_addr) { - bgp_size_t total; - + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + const bgp_size_t length = args->length; + if (length != 8) { - zlog (peer->log, LOG_ERR, "New Aggregator length is not 8 [%d]", length); - return bgp_attr_malformed (peer, BGP_ATTR_AS4_AGGREGATOR, flag, + zlog (peer->log, LOG_ERR, "New Aggregator length is not 8 [%d]", + length); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - NULL, 0); + 0); } - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - /* Flags check. */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_AS4_AGGREGATOR, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_AS4_AGGREGATOR, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); *as4_aggregator_as = stream_getl (peer->ibuf); as4_aggregator_addr->s_addr = stream_get_ipv4 (peer->ibuf); @@ -1296,7 +1245,8 @@ bgp_attr_as4_aggregator (struct peer *peer, bgp_size_t length, /* Munge Aggregator and New-Aggregator, AS_PATH and NEW_AS_PATH. */ static bgp_attr_parse_ret_t -bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr, u_char flag, +bgp_attr_munge_as4_attrs (struct peer *const peer, + struct attr *const attr, struct aspath *as4_path, as_t as4_aggregator, struct in_addr *as4_aggregator_addr) { @@ -1325,24 +1275,6 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr, u_char flag, return BGP_ATTR_PARSE_PROCEED; } - if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH)) - && !(attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS_PATH)))) - { - /* Hu? This is not supposed to happen at all! - * got as4_path and no aspath, - * This should already - * have been handled by 'well known attributes missing' - * But... yeah, paranoia - * Take this as a "malformed attribute" - */ - zlog (peer->log, LOG_ERR, - "%s BGP not AS4 capable peer sent AS4_PATH but" - " no AS_PATH, cant do anything here", peer->host); - return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag, - BGP_NOTIFY_UPDATE_MAL_ATTR, - NULL, 0); - } - /* We have a asn16 peer. First, look for AS4_AGGREGATOR * because that may override AS4_PATH */ @@ -1410,11 +1342,11 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr, u_char flag, /* Community attribute. */ static bgp_attr_parse_ret_t -bgp_attr_community (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, u_char *startp) +bgp_attr_community (struct bgp_attr_parser_args *args) { - bgp_size_t total - = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + const bgp_size_t length = args->length; if (length == 0) { @@ -1422,12 +1354,6 @@ bgp_attr_community (struct peer *peer, bgp_size_t length, return BGP_ATTR_PARSE_PROCEED; } - /* Flags check. */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_COMMUNITIES, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_COMMUNITIES, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); - attr->community = community_parse ((u_int32_t *)stream_pnt (peer->ibuf), length); @@ -1435,9 +1361,9 @@ bgp_attr_community (struct peer *peer, bgp_size_t length, stream_forward_getp (peer->ibuf, length); if (!attr->community) - return bgp_attr_malformed (peer, BGP_ATTR_COMMUNITIES, flag, + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR, - startp, total); + args->total); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES); @@ -1446,25 +1372,20 @@ bgp_attr_community (struct peer *peer, bgp_size_t length, /* Originator ID attribute. */ static bgp_attr_parse_ret_t -bgp_attr_originator_id (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, u_char *startp) +bgp_attr_originator_id (struct bgp_attr_parser_args *args) { - bgp_size_t total; - - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - /* Flag checks. */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_ORIGINATOR_ID, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_ORIGINATOR_ID, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + const bgp_size_t length = args->length; + /* Length check. */ if (length != 4) { zlog (peer->log, LOG_ERR, "Bad originator ID length %d", length); - return bgp_attr_malformed (peer, BGP_ATTR_ORIGINATOR_ID, flag, + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - startp, total); + args->total); } (bgp_attr_extra_get (attr))->originator_id.s_addr @@ -1477,25 +1398,19 @@ bgp_attr_originator_id (struct peer *peer, bgp_size_t length, /* Cluster list attribute. */ static bgp_attr_parse_ret_t -bgp_attr_cluster_list (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, u_char *startp) +bgp_attr_cluster_list (struct bgp_attr_parser_args *args) { - bgp_size_t total; - - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - /* Flag checks. */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_CLUSTER_LIST, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_CLUSTER_LIST, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + const bgp_size_t length = args->length; + /* Check length. */ if (length % 4) { zlog (peer->log, LOG_ERR, "Bad cluster list length %d", length); - return bgp_attr_malformed (peer, BGP_ATTR_CLUSTER_LIST, flag, - BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, - startp, total); + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, + args->total); } (bgp_attr_extra_get (attr))->cluster @@ -1511,8 +1426,8 @@ bgp_attr_cluster_list (struct peer *peer, bgp_size_t length, /* Multiprotocol reachability information parse. */ int -bgp_mp_reach_parse (struct peer *peer, const bgp_size_t length, - struct attr *attr, const u_char flag, u_char *startp, struct bgp_nlri *mp_update) +bgp_mp_reach_parse (struct bgp_attr_parser_args *args, + struct bgp_nlri *mp_update) { afi_t afi; safi_t safi; @@ -1520,15 +1435,11 @@ bgp_mp_reach_parse (struct peer *peer, const bgp_size_t length, size_t start; int ret; struct stream *s; + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + const bgp_size_t length = args->length; struct attr_extra *attre = bgp_attr_extra_get(attr); - bgp_size_t total; - - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - /* Flag checks. */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_MP_REACH_NLRI, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_MP_REACH_NLRI, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); + /* Set end of packet. */ s = BGP_INPUT(peer); start = stream_get_getp(s); @@ -1647,8 +1558,7 @@ bgp_mp_reach_parse (struct peer *peer, const bgp_size_t length, /* Multiprotocol unreachable parse */ int -bgp_mp_unreach_parse (struct peer *peer, const bgp_size_t length, - const u_char flag, u_char *startp, +bgp_mp_unreach_parse (struct bgp_attr_parser_args *args, struct bgp_nlri *mp_withdraw) { struct stream *s; @@ -1656,14 +1566,8 @@ bgp_mp_unreach_parse (struct peer *peer, const bgp_size_t length, safi_t safi; u_int16_t withdraw_len; int ret; - bgp_size_t total; - - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - /* Flag checks. */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_MP_UNREACH_NLRI, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_MP_UNREACH_NLRI, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); + struct peer *const peer = args->peer; + const bgp_size_t length = args->length; s = peer->ibuf; @@ -1695,11 +1599,11 @@ bgp_mp_unreach_parse (struct peer *peer, const bgp_size_t length, /* Extended Community attribute. */ static bgp_attr_parse_ret_t -bgp_attr_ext_communities (struct peer *peer, bgp_size_t length, - struct attr *attr, u_char flag, u_char *startp) +bgp_attr_ext_communities (struct bgp_attr_parser_args *args) { - bgp_size_t total - = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + const bgp_size_t length = args->length; if (length == 0) { @@ -1709,21 +1613,15 @@ bgp_attr_ext_communities (struct peer *peer, bgp_size_t length, return BGP_ATTR_PARSE_PROCEED; } - /* Flags check. */ - if (bgp_attr_flag_invalid (peer, BGP_ATTR_EXT_COMMUNITIES, flag)) - return bgp_attr_malformed (peer, BGP_ATTR_EXT_COMMUNITIES, flag, - BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, - startp, total); - (bgp_attr_extra_get (attr))->ecommunity = ecommunity_parse ((u_int8_t *)stream_pnt (peer->ibuf), length); /* XXX: fix ecommunity_parse to use stream API */ stream_forward_getp (peer->ibuf, length); if (!attr->extra->ecommunity) - return bgp_attr_malformed (peer, BGP_ATTR_EXT_COMMUNITIES, - flag, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR, - startp, total); + return bgp_attr_malformed (args, + BGP_NOTIFY_UPDATE_OPT_ATTR_ERR, + args->total); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES); @@ -1732,12 +1630,18 @@ bgp_attr_ext_communities (struct peer *peer, bgp_size_t length, /* BGP unknown attribute treatment. */ static bgp_attr_parse_ret_t -bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag, - u_char type, bgp_size_t length, u_char *startp) +bgp_attr_unknown (struct bgp_attr_parser_args *args) { bgp_size_t total; struct transit *transit; struct attr_extra *attre; + struct peer *const peer = args->peer; + struct attr *const attr = args->attr; + u_char *const startp = args->startp; + const u_char type = args->type; + const u_char flag = args->flags; + const bgp_size_t length = args->length; + if (BGP_DEBUG (normal, NORMAL)) zlog_debug ("%s Unknown attribute is received (type %d, length %d)", @@ -1750,18 +1654,15 @@ bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag, /* Forward read pointer of input stream. */ stream_forward_getp (peer->ibuf, length); - /* Adjest total length to include type and length. */ - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - /* If any of the mandatory well-known attributes are not recognized, then the Error Subcode is set to Unrecognized Well-known Attribute. The Data field contains the unrecognized attribute (type, length and value). */ if (!CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)) { - return bgp_attr_malformed (peer, type, flag, + return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_UNREC_ATTR, - startp, total); + args->total); } /* Unrecognized non-transitive optional attributes must be quietly @@ -1858,7 +1759,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); return BGP_ATTR_PARSE_ERROR; } - + /* Check extended attribue length bit. */ if (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN)) length = stream_getw (BGP_INPUT (peer)); @@ -1898,59 +1799,79 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); return BGP_ATTR_PARSE_ERROR; } + + struct bgp_attr_parser_args attr_args = { + .peer = peer, + .length = length, + .attr = attr, + .type = type, + .flags = flag, + .startp = startp, + .total = attr_endp - startp, + }; + + + /* If any recognized attribute has Attribute Flags that conflict + with the Attribute Type Code, then the Error Subcode is set to + Attribute Flags Error. The Data field contains the erroneous + attribute (type, length and value). */ + if (bgp_attr_flag_invalid (&attr_args)) + return bgp_attr_malformed (&attr_args, + BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, + attr_args.total); /* OK check attribute and store it's value. */ switch (type) { case BGP_ATTR_ORIGIN: - ret = bgp_attr_origin (peer, length, attr, flag, startp); + ret = bgp_attr_origin (&attr_args); break; case BGP_ATTR_AS_PATH: - ret = bgp_attr_aspath (peer, length, attr, flag, startp); + ret = bgp_attr_aspath (&attr_args); break; case BGP_ATTR_AS4_PATH: - ret = bgp_attr_as4_path (peer, length, attr, flag, startp, &as4_path); + ret = bgp_attr_as4_path (&attr_args, &as4_path); break; case BGP_ATTR_NEXT_HOP: - ret = bgp_attr_nexthop (peer, length, attr, flag, startp); + ret = bgp_attr_nexthop (&attr_args); break; case BGP_ATTR_MULTI_EXIT_DISC: - ret = bgp_attr_med (peer, length, attr, flag, startp); + ret = bgp_attr_med (&attr_args); break; case BGP_ATTR_LOCAL_PREF: - ret = bgp_attr_local_pref (peer, length, attr, flag, startp); + ret = bgp_attr_local_pref (&attr_args); break; case BGP_ATTR_ATOMIC_AGGREGATE: - ret = bgp_attr_atomic (peer, length, attr, flag, startp); + ret = bgp_attr_atomic (&attr_args); break; case BGP_ATTR_AGGREGATOR: - ret = bgp_attr_aggregator (peer, length, attr, flag, startp); + ret = bgp_attr_aggregator (&attr_args); break; case BGP_ATTR_AS4_AGGREGATOR: - ret = bgp_attr_as4_aggregator (peer, length, attr, flag, - &as4_aggregator, - &as4_aggregator_addr, startp); + ret = bgp_attr_as4_aggregator (&attr_args, + &as4_aggregator, + &as4_aggregator_addr); break; case BGP_ATTR_COMMUNITIES: - ret = bgp_attr_community (peer, length, attr, flag, startp); + ret = bgp_attr_community (&attr_args); break; case BGP_ATTR_ORIGINATOR_ID: - ret = bgp_attr_originator_id (peer, length, attr, flag, startp); + ret = bgp_attr_originator_id (&attr_args); break; case BGP_ATTR_CLUSTER_LIST: - ret = bgp_attr_cluster_list (peer, length, attr, flag, startp); + ret = bgp_attr_cluster_list (&attr_args); break; case BGP_ATTR_MP_REACH_NLRI: - ret = bgp_mp_reach_parse (peer, length, attr, flag, startp, mp_update); + ret = bgp_mp_reach_parse (&attr_args, mp_update); break; case BGP_ATTR_MP_UNREACH_NLRI: - ret = bgp_mp_unreach_parse (peer, length, flag, startp, mp_withdraw); + ret = bgp_mp_unreach_parse (&attr_args, mp_withdraw); break; case BGP_ATTR_EXT_COMMUNITIES: - ret = bgp_attr_ext_communities (peer, length, attr, flag, startp); + ret = bgp_attr_ext_communities (&attr_args); break; default: - ret = bgp_attr_unknown (peer, attr, flag, type, length, startp); + ret = bgp_attr_unknown (&attr_args); break; } @@ -2020,7 +1941,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, * all attributes first, including these 32bit ones, and now, * afterwards, we look what and if something is to be done for as4. */ - if (bgp_attr_munge_as4_attrs (peer, attr, flag, as4_path, + if (bgp_attr_munge_as4_attrs (peer, attr, as4_path, as4_aggregator, &as4_aggregator_addr)) { if (as4_path) @@ -2052,7 +1973,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, */ if (attr->flag & (ATTR_FLAG_BIT(BGP_ATTR_AS_PATH))) { - ret = bgp_attr_aspath_check (peer, attr, flag); + ret = bgp_attr_aspath_check (peer, attr); if (ret != BGP_ATTR_PARSE_PROCEED) return ret; } diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index e6300740..df87c863 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -179,10 +179,19 @@ extern void cluster_unintern (struct cluster_list *); /* Transit attribute prototypes. */ void transit_unintern (struct transit *); -/* Exported for unit-test purposes only */ -extern int bgp_mp_reach_parse (struct peer *, const bgp_size_t, struct attr *, - const u_char, u_char *, struct bgp_nlri *); -extern int bgp_mp_unreach_parse (struct peer *, const bgp_size_t, const u_char, - u_char *, struct bgp_nlri *); +/* Below exported for unit-test purposes only */ +struct bgp_attr_parser_args { + struct peer *peer; + bgp_size_t length; /* attribute data length; */ + bgp_size_t total; /* total length, inc header */ + struct attr *attr; + u_int8_t type; + u_int8_t flags; + u_char *startp; +}; +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 *); #endif /* _QUAGGA_BGP_ATTR_H */ -- cgit v1.2.1