From e531d4a662749fdd2a2f83f16e0b1cd252b23ea1 Mon Sep 17 00:00:00 2001 From: Denis Ovsienko Date: Sat, 24 Sep 2011 13:20:43 +0400 Subject: bgpd: add missing "partial" flag checks (BZ#676) ORIGIN handling function used to have "partial" bit check and recent commits added it for NEXT_HOP, MULTI_EXIT_DISC and ATOMIC_AGGREGATE cases. This commit adds "partial" check for AS_PATH and LOCAL_PREF cases, which should leave attributes 1 through 6 inclusive completely covered with attribute flags checks. * bgp_attr.c * bgp_attr_origin(): use bit-by-bit checks for better diagnostics * bgp_attr_aspath(): add flag check * bgp_attr_local_pref(): idem --- bgpd/bgp_attr.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 8 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 251f1568..7974477b 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -712,12 +712,32 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length, 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 (flag != BGP_ATTR_FLAG_TRANS) + if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)) { - zlog (peer->log, LOG_ERR, - "Origin attribute flag isn't transitive %d", flag); - bgp_notify_send_with_data (peer, - BGP_NOTIFY_UPDATE_ERR, + zlog (peer->log, LOG_ERR, + "ORIGIN attribute must not be flagged as \"optional\" (%u)", flag); + bgp_notify_send_with_data (peer, + BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, + startp, total); + return -1; + } + if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS)) + { + zlog (peer->log, LOG_ERR, + "ORIGIN attribute must be flagged as \"transitive\" (%u)", flag); + bgp_notify_send_with_data (peer, + BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, + startp, total); + return -1; + } + if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL)) + { + zlog (peer->log, LOG_ERR, + "ORIGIN attribute must not be flagged as \"partial\" (%u)", flag); + bgp_notify_send_with_data (peer, + BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, startp, total); return -1; @@ -780,15 +800,28 @@ bgp_attr_aspath (struct peer *peer, bgp_size_t length, { u_char require ; struct aspath *asp ; + bgp_size_t total; + + total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); /* Check the attribute flags */ + if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL)) + { + zlog (peer->log, LOG_ERR, + "AS_PATH attribute must not be flagged as \"partial\" (%u)", flag); + bgp_notify_send_with_data (peer, + BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, + startp, total); + return NULL; + } + require = as4_path ? BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS : BGP_ATTR_FLAG_TRANS ; if ((flag & (BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS)) != require) { const char* path_type ; - bgp_size_t total; path_type = as4_path ? "AS4_PATH" : "AS_PATH" ; @@ -801,8 +834,6 @@ bgp_attr_aspath (struct peer *peer, bgp_size_t length, "%s attribute flag must %sbe optional %d", path_type, (flag & BGP_ATTR_FLAG_OPTIONAL) ? "not " : "", flag) ; - total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, @@ -1062,6 +1093,16 @@ bgp_attr_local_pref (struct peer *peer, bgp_size_t length, startp, total); return -1; } + if (CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL)) + { + zlog (peer->log, LOG_ERR, + "LOCAL_PREF attribute must not be flagged as \"partial\" (%u)", flag); + bgp_notify_send_with_data (peer, + BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, + startp, total); + return -1; + } /* If it is contained in an UPDATE message that is received from an external peer, then this attribute MUST be ignored by the -- cgit v1.2.1