proxmox-mirrors/mirror_frr
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_frr synced 2025-08-08 22:22:16 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

bgpd: Implement revised error handling for partial optional/trans. attributes

Browse Source 
* BGP error handling generally boils down to "reset session". This was fine
  when all BGP speakers pretty much understood all BGP messages. However
  the increasing deployment of new attribute types has shown this approach
  to cause problems, in particular where a new attribute type is "tunneled"
  over some speakers which do not understand it, and then arrives at a speaker
  which does but considers it malformed (e.g. corruption along the way, or
  because of early implementation bugs/interop issues).

  To mitigate this drafts before the IDR (likely to be adopted) propose to
  treat errors in partial (i.e.  not understood by neighbour), optional
  transitive attributes, when received from eBGP peers, as withdrawing only
  the NLRIs in the affected UPDATE, rather than causing the entire session
  to be reset.  See:

   http://tools.ietf.org/html/draft-scudder-idr-optional-transitive

* bgp_aspath.c: (assegments_parse) Replace the "NULL means valid, 0-length
  OR an error" return value with an error code - instead taking
  pointer to result structure as arg.
  (aspath_parse) adjust to suit previous change, but here NULL really
  does mean error in the external interface.
* bgp_attr.h (bgp_attr_parse) use an explictly typed and enumerated
  value to indicate return result.
  (bgp_attr_unintern_sub) cleans up just the members of an attr, but not the
  attr itself, for benefit of those who use a stack-local attr.
* bgp_attr.c: (bgp_attr_unintern_sub) split out from bgp_attr_unintern
  (bgp_attr_unintern) as previous.
  (bgp_attr_malformed) helper function to centralise decisions on how to
  handle errors in attributes.
  (bgp_attr_{aspathlimit,origin,etc..}) Use bgp_attr_malformed.
  (bgp_attr_aspathlimit) Subcode for error specifc to this attr should be
  BGP_NOTIFY_UPDATE_OPT_ATTR_ERR.
  (bgp_attr_as4_path) be more rigorous about checks, ala bgp_attr_as_path.
  (bgp_attr_parse) Adjust to deal with the additional error level that
  bgp_attr_ parsers can raise, and also similarly return appropriate
  error back up to (bgp_update_receive). Try to avoid leaking as4_path.
* bgp_packet.c: (bgp_update_receive) Adjust to deal with BGP_ATTR_PARSE_WITHDRAW
  error level from bgp_attr_parse, which should lead to a withdraw, by
  making the attribute parameter in call to (bgp_nlri_parse) conditional
  on the error, so the update case morphs also into a withdraw.
  Use bgp_attr_unintern_sub from above, instead of doing this itself.
  Fix error case returns which were not calling bgp_attr_unintern_sub
  and probably leaking memory.
* tests/aspath_test.c: Fix to work for null return with bad segments
This commit is contained in:
Paul Jakma 2010-11-23 16:35:42 +00:00
parent c112af27ed
commit b881c7074b
5 changed files with 393 additions and 239 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
bgpd/bgp_aspath.c
View File

@ -674,8 +674,9 @@ aspath_hash_alloc (void *arg)
} }
/* parse as-segment byte stream in struct assegment */ /* parse as-segment byte stream in struct assegment */
static struct assegment * static int
assegments_parse (struct stream *s, size_t length, int use32bit) assegments_parse (struct stream *s, size_t length,
struct assegment **result, int use32bit)
{ {
struct assegment_header segh; struct assegment_header segh;
struct assegment *seg, *prev = NULL, *head = NULL; struct assegment *seg, *prev = NULL, *head = NULL;
@ -683,7 +684,7 @@ assegments_parse (struct stream *s, size_t length, int use32bit)
/* empty aspath (ie iBGP or somesuch) */ /* empty aspath (ie iBGP or somesuch) */
if (length == 0) if (length == 0)
return NULL; return 0;
if (BGP_DEBUG (as4, AS4_SEGMENT)) if (BGP_DEBUG (as4, AS4_SEGMENT))
zlog_debug ("[AS4SEG] Parse aspath segment: got total byte length %lu", zlog_debug ("[AS4SEG] Parse aspath segment: got total byte length %lu",
@ -692,7 +693,7 @@ assegments_parse (struct stream *s, size_t length, int use32bit)
if ((STREAM_READABLE(s) < length) if ((STREAM_READABLE(s) < length)
|| (STREAM_READABLE(s) < AS_HEADER_SIZE) || (STREAM_READABLE(s) < AS_HEADER_SIZE)
|| (length % AS16_VALUE_SIZE )) || (length % AS16_VALUE_SIZE ))
return NULL; return -1;
while (bytes < length) while (bytes < length)
{ {
@ -703,7 +704,7 @@ assegments_parse (struct stream *s, size_t length, int use32bit)
{ {
if (head) if (head)
assegment_free_all (head); assegment_free_all (head);
return NULL; return -1;
} }
/* softly softly, get the header first on its own */ /* softly softly, get the header first on its own */
@ -729,7 +730,7 @@ assegments_parse (struct stream *s, size_t length, int use32bit)
{ {
if (head) if (head)
assegment_free_all (head); assegment_free_all (head);
return NULL; return -1;
} }
switch (segh.type) switch (segh.type)
@ -742,7 +743,7 @@ assegments_parse (struct stream *s, size_t length, int use32bit)
default: default:
if (head) if (head)
assegment_free_all (head); assegment_free_all (head);
return NULL; return -1;
} }
/* now its safe to trust lengths */ /* now its safe to trust lengths */
@ -765,12 +766,16 @@ assegments_parse (struct stream *s, size_t length, int use32bit)
prev = seg; prev = seg;
} }
return assegment_normalise (head); *result = assegment_normalise (head);
return 0;
} }
/* AS path parse function. pnt is a pointer to byte stream and length /* AS path parse function. pnt is a pointer to byte stream and length
is length of byte stream. If there is same AS path in the the AS is length of byte stream. If there is same AS path in the the AS
path hash then return it else make new AS path structure. */ path hash then return it else make new AS path structure.
On error NULL is returned.
*/
struct aspath * struct aspath *
aspath_parse (struct stream *s, size_t length, int use32bit) aspath_parse (struct stream *s, size_t length, int use32bit)
{ {
@ -787,7 +792,8 @@ aspath_parse (struct stream *s, size_t length, int use32bit)
return NULL; return NULL;
memset (&as, 0, sizeof (struct aspath)); memset (&as, 0, sizeof (struct aspath));
as.segments = assegments_parse (s, length, use32bit); if (assegments_parse (s, length, &as.segments, use32bit) < 0)
return NULL;
/* If already same aspath exist then return it. */ /* If already same aspath exist then return it. */
find = hash_get (ashash, &as, aspath_hash_alloc); find = hash_get (ashash, &as, aspath_hash_alloc);

445
bgpd/bgp_attr.c
View File

@ -618,26 +618,50 @@ bgp_attr_aggregate_intern (struct bgp *bgp, u_char origin,
return new; return new;
} }
/* Unintern just the sub-components of the attr, but not the attr */
void
bgp_attr_unintern_sub (struct attr *attr)
{
/* aspath refcount shoud be decrement. */
if (attr->aspath)
aspath_unintern (&attr->aspath);
UNSET_FLAG(attr->flag, BGP_ATTR_AS_PATH);
if (attr->community)
community_unintern (&attr->community);
UNSET_FLAG(attr->flag, BGP_ATTR_COMMUNITIES);
if (attr->extra)
{
if (attr->extra->ecommunity)
ecommunity_unintern (&attr->extra->ecommunity);
UNSET_FLAG(attr->flag, BGP_ATTR_EXT_COMMUNITIES);
if (attr->extra->cluster)
cluster_unintern (attr->extra->cluster);
UNSET_FLAG(attr->flag, BGP_ATTR_CLUSTER_LIST);
if (attr->extra->transit)
transit_unintern (attr->extra->transit);
}
}
/* Free bgp attribute and aspath. */ /* Free bgp attribute and aspath. */
void void
bgp_attr_unintern (struct attr **attr) bgp_attr_unintern (struct attr **attr)
{ {
struct attr *ret; struct attr *ret;
struct aspath *aspath; struct attr tmp;
struct community *community;
struct ecommunity *ecommunity = NULL;
struct cluster_list *cluster = NULL;
struct transit *transit = NULL;
/* Decrement attribute reference. */ /* Decrement attribute reference. */
(*attr)->refcnt--; (*attr)->refcnt--;
aspath = (*attr)->aspath;
community = (*attr)->community; tmp = *(*attr);
if ((*attr)->extra) if ((*attr)->extra)
{ {
ecommunity = (*attr)->extra->ecommunity; tmp.extra = bgp_attr_extra_new ();
cluster = (*attr)->extra->cluster; memcpy (tmp.extra, (*attr)->extra, sizeof (struct attr_extra));
transit = (*attr)->extra->transit;
} }
/* If reference becomes zero then free attribute object. */ /* If reference becomes zero then free attribute object. */
@ -650,17 +674,7 @@ bgp_attr_unintern (struct attr **attr)
*attr = NULL; *attr = NULL;
} }
/* aspath refcount shoud be decrement. */ bgp_attr_unintern_sub (&tmp);
if (aspath)
aspath_unintern (&aspath);
if (community)
community_unintern (&community);
if (ecommunity)
ecommunity_unintern (&ecommunity);
if (cluster)
cluster_unintern (cluster);
if (transit)
transit_unintern (transit);
} }
void void
@ -683,8 +697,69 @@ bgp_attr_flush (struct attr *attr)
} }
} }
/* Implement draft-scudder-idr-optional-transitive behaviour and
* avoid resetting sessions for malformed attributes which are
* are partial/optional and hence where the error likely was not
* 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)
{
/* Only relax error handling for eBGP peers */
if (peer_sort (peer) != BGP_PEER_EBGP)
{
bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, subcode,
startp, length);
return BGP_ATTR_PARSE_ERROR;
}
switch (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
* normal.
*/
case BGP_ATTR_AS4_AGGREGATOR:
case BGP_ATTR_AGGREGATOR:
case BGP_ATTR_ATOMIC_AGGREGATE:
return BGP_ATTR_PARSE_PROCEED;
/* Core attributes, particularly ones which may influence route
* selection should always cause session resets
*/
case BGP_ATTR_ORIGIN:
case BGP_ATTR_AS_PATH:
case BGP_ATTR_NEXT_HOP:
case BGP_ATTR_MULTI_EXIT_DISC:
case BGP_ATTR_LOCAL_PREF:
case BGP_ATTR_COMMUNITIES:
case BGP_ATTR_ORIGINATOR_ID:
case BGP_ATTR_CLUSTER_LIST:
case BGP_ATTR_MP_REACH_NLRI:
case BGP_ATTR_MP_UNREACH_NLRI:
case BGP_ATTR_EXT_COMMUNITIES:
bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, subcode,
startp, length);
return BGP_ATTR_PARSE_ERROR;
}
/* 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 (flag, BGP_ATTR_FLAG_TRANS)
&& CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)
&& CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
return BGP_ATTR_PARSE_WITHDRAW;
/* default to reset */
return BGP_ATTR_PARSE_ERROR;
}
/* Get origin attribute of the update message. */ /* Get origin attribute of the update message. */
static int static bgp_attr_parse_ret_t
bgp_attr_origin (struct peer *peer, bgp_size_t length, bgp_attr_origin (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag, u_char *startp) struct attr *attr, u_char flag, u_char *startp)
{ {
@ -702,11 +777,9 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length,
{ {
zlog (peer->log, LOG_ERR, zlog (peer->log, LOG_ERR,
"Origin attribute flag isn't transitive %d", flag); "Origin attribute flag isn't transitive %d", flag);
bgp_notify_send_with_data (peer, return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total); startp, total);
return -1;
} }
/* If any recognized attribute has Attribute Length that conflicts /* If any recognized attribute has Attribute Length that conflicts
@ -718,10 +791,9 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length,
{ {
zlog (peer->log, LOG_ERR, "Origin attribute length is not one %d", zlog (peer->log, LOG_ERR, "Origin attribute length is not one %d",
length); length);
bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
startp, total); startp, total);
return -1;
} }
/* Fetch origin attribute. */ /* Fetch origin attribute. */
@ -736,12 +808,9 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length,
{ {
zlog (peer->log, LOG_ERR, "Origin attribute value is invalid %d", zlog (peer->log, LOG_ERR, "Origin attribute value is invalid %d",
attr->origin); attr->origin);
return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
bgp_notify_send_with_data (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_INVAL_ORIGIN, BGP_NOTIFY_UPDATE_INVAL_ORIGIN,
startp, total); startp, total);
return -1;
} }
/* Set oring attribute flag. */ /* Set oring attribute flag. */
@ -766,11 +835,9 @@ bgp_attr_aspath (struct peer *peer, bgp_size_t length,
{ {
zlog (peer->log, LOG_ERR, zlog (peer->log, LOG_ERR,
"As-Path attribute flag isn't transitive %d", flag); "As-Path attribute flag isn't transitive %d", flag);
bgp_notify_send_with_data (peer, return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total); startp, total);
return -1;
} }
/* /*
@ -783,21 +850,22 @@ bgp_attr_aspath (struct peer *peer, bgp_size_t length,
/* In case of IBGP, length will be zero. */ /* In case of IBGP, length will be zero. */
if (! attr->aspath) if (! attr->aspath)
{ {
zlog (peer->log, LOG_ERR, "Malformed AS path length is %d", length); zlog (peer->log, LOG_ERR,
bgp_notify_send (peer, "Malformed AS path from %s, length is %d",
BGP_NOTIFY_UPDATE_ERR, peer->host, length);
BGP_NOTIFY_UPDATE_MAL_AS_PATH); return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
return -1; BGP_NOTIFY_UPDATE_MAL_AS_PATH,
NULL, 0);
} }
/* Set aspath attribute flag. */ /* Set aspath attribute flag. */
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS_PATH); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS_PATH);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
static int bgp_attr_aspath_check( struct peer *peer, static bgp_attr_parse_ret_t
struct attr *attr) bgp_attr_aspath_check (struct peer *peer, struct attr *attr, u_char flag)
{ {
/* These checks were part of bgp_attr_aspath, but with /* These checks were part of bgp_attr_aspath, but with
* as4 we should to check aspath things when * as4 we should to check aspath things when
@ -816,10 +884,9 @@ static int bgp_attr_aspath_check( struct peer *peer,
(peer_sort (peer) == BGP_PEER_EBGP && aspath_confed_check (attr->aspath))) (peer_sort (peer) == BGP_PEER_EBGP && aspath_confed_check (attr->aspath)))
{ {
zlog (peer->log, LOG_ERR, "Malformed AS path from %s", peer->host); zlog (peer->log, LOG_ERR, "Malformed AS path from %s", peer->host);
bgp_notify_send (peer, return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_MAL_AS_PATH,
BGP_NOTIFY_UPDATE_MAL_AS_PATH); NULL, 0);
return -1;
} }
/* First AS check for EBGP. */ /* First AS check for EBGP. */
@ -830,10 +897,9 @@ static int bgp_attr_aspath_check( struct peer *peer,
{ {
zlog (peer->log, LOG_ERR, zlog (peer->log, LOG_ERR,
"%s incorrect first AS (must be %u)", peer->host, peer->as); "%s incorrect first AS (must be %u)", peer->host, peer->as);
bgp_notify_send (peer, return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_MAL_AS_PATH,
BGP_NOTIFY_UPDATE_MAL_AS_PATH); NULL, 0);
return -1;
} }
} }
@ -847,27 +913,53 @@ static int bgp_attr_aspath_check( struct peer *peer,
attr->aspath = aspath_intern (aspath); attr->aspath = aspath_intern (aspath);
} }
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* Parse AS4 path information. This function is another wrapper of /* Parse AS4 path information. This function is another wrapper of
aspath_parse. */ aspath_parse. */
static int static int
bgp_attr_as4_path (struct peer *peer, bgp_size_t length, bgp_attr_as4_path (struct peer *peer, bgp_size_t length,
struct attr *attr, struct aspath **as4_path) struct attr *attr, u_char flag, u_char *startp,
struct aspath **as4_path)
{ {
bgp_size_t total;
total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
/* Flag check. */
if (!CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)
|| !CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
{
zlog (peer->log, LOG_ERR,
"As4-Path attribute flag isn't optional/transitive %d", flag);
return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total);
}
*as4_path = aspath_parse (peer->ibuf, length, 1); *as4_path = aspath_parse (peer->ibuf, length, 1);
/* In case of IBGP, length will be zero. */
if (!*as4_path)
{
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,
BGP_NOTIFY_UPDATE_MAL_AS_PATH,
NULL, 0);
}
/* Set aspath attribute flag. */ /* Set aspath attribute flag. */
if (as4_path) if (as4_path)
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* Nexthop attribute. */ /* Nexthop attribute. */
static int static bgp_attr_parse_ret_t
bgp_attr_nexthop (struct peer *peer, bgp_size_t length, bgp_attr_nexthop (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag, u_char *startp) struct attr *attr, u_char flag, u_char *startp)
{ {
@ -881,11 +973,9 @@ bgp_attr_nexthop (struct peer *peer, bgp_size_t length,
{ {
zlog (peer->log, LOG_ERR, zlog (peer->log, LOG_ERR,
"Origin attribute flag isn't transitive %d", flag); "Origin attribute flag isn't transitive %d", flag);
bgp_notify_send_with_data (peer, return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
startp, total); startp, total);
return -1;
} }
/* Check nexthop attribute length. */ /* Check nexthop attribute length. */
@ -894,21 +984,19 @@ bgp_attr_nexthop (struct peer *peer, bgp_size_t length,
zlog (peer->log, LOG_ERR, "Nexthop attribute length isn't four [%d]", zlog (peer->log, LOG_ERR, "Nexthop attribute length isn't four [%d]",
length); length);
bgp_notify_send_with_data (peer, return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
startp, total); startp, total);
return -1;
} }
attr->nexthop.s_addr = stream_get_ipv4 (peer->ibuf); attr->nexthop.s_addr = stream_get_ipv4 (peer->ibuf);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* MED atrribute. */ /* MED atrribute. */
static int static bgp_attr_parse_ret_t
bgp_attr_med (struct peer *peer, bgp_size_t length, bgp_attr_med (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag, u_char *startp) struct attr *attr, u_char flag, u_char *startp)
{ {
@ -922,22 +1010,20 @@ bgp_attr_med (struct peer *peer, bgp_size_t length,
zlog (peer->log, LOG_ERR, zlog (peer->log, LOG_ERR,
"MED attribute length isn't four [%d]", length); "MED attribute length isn't four [%d]", length);
bgp_notify_send_with_data (peer, return bgp_attr_malformed (peer, BGP_ATTR_MULTI_EXIT_DISC, flag,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
startp, total); startp, total);
return -1;
} }
attr->med = stream_getl (peer->ibuf); attr->med = stream_getl (peer->ibuf);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* Local preference attribute. */ /* Local preference attribute. */
static int static bgp_attr_parse_ret_t
bgp_attr_local_pref (struct peer *peer, bgp_size_t length, bgp_attr_local_pref (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag) struct attr *attr, u_char flag)
{ {
@ -947,7 +1033,7 @@ bgp_attr_local_pref (struct peer *peer, bgp_size_t length,
if (peer_sort (peer) == BGP_PEER_EBGP) if (peer_sort (peer) == BGP_PEER_EBGP)
{ {
stream_forward_getp (peer->ibuf, length); stream_forward_getp (peer->ibuf, length);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
if (length == 4) if (length == 4)
@ -958,7 +1044,7 @@ bgp_attr_local_pref (struct peer *peer, bgp_size_t length,
/* Set atomic aggregate flag. */ /* Set atomic aggregate flag. */
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* Atomic aggregate. */ /* Atomic aggregate. */
@ -970,16 +1056,15 @@ bgp_attr_atomic (struct peer *peer, bgp_size_t length,
{ {
zlog (peer->log, LOG_ERR, "Bad atomic aggregate length %d", length); zlog (peer->log, LOG_ERR, "Bad atomic aggregate length %d", length);
bgp_notify_send (peer, return bgp_attr_malformed (peer, BGP_ATTR_ATOMIC_AGGREGATE, flag,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); NULL, 0);
return -1;
} }
/* Set atomic aggregate flag. */ /* Set atomic aggregate flag. */
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ATOMIC_AGGREGATE); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ATOMIC_AGGREGATE);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* Aggregator attribute */ /* Aggregator attribute */
@ -991,17 +1076,16 @@ bgp_attr_aggregator (struct peer *peer, bgp_size_t length,
struct attr_extra *attre = bgp_attr_extra_get (attr); struct attr_extra *attre = bgp_attr_extra_get (attr);
/* peer with AS4 will send 4 Byte AS, peer without will send 2 Byte */ /* peer with AS4 will send 4 Byte AS, peer without will send 2 Byte */
if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV ) ) if (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV))
wantedlen = 8; wantedlen = 8;
if (length != wantedlen) if (length != wantedlen)
{ {
zlog (peer->log, LOG_ERR, "Aggregator length is not %d [%d]", wantedlen, length); zlog (peer->log, LOG_ERR, "Aggregator length is not %d [%d]", wantedlen, length);
bgp_notify_send (peer, return bgp_attr_malformed (peer, BGP_ATTR_AGGREGATOR, flag,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); NULL, 0);
return -1;
} }
if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV ) ) if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV ) )
@ -1013,36 +1097,35 @@ bgp_attr_aggregator (struct peer *peer, bgp_size_t length,
/* Set atomic aggregate flag. */ /* Set atomic aggregate flag. */
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* New Aggregator attribute */ /* New Aggregator attribute */
static int static bgp_attr_parse_ret_t
bgp_attr_as4_aggregator (struct peer *peer, bgp_size_t length, bgp_attr_as4_aggregator (struct peer *peer, bgp_size_t length,
struct attr *attr, as_t *as4_aggregator_as, struct attr *attr, u_char flag,
as_t *as4_aggregator_as,
struct in_addr *as4_aggregator_addr) struct in_addr *as4_aggregator_addr)
{ {
if (length != 8) if (length != 8)
{ {
zlog (peer->log, LOG_ERR, "New Aggregator length is not 8 [%d]", length); zlog (peer->log, LOG_ERR, "New Aggregator length is not 8 [%d]", length);
return bgp_attr_malformed (peer, BGP_ATTR_AS4_AGGREGATOR, flag,
bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
BGP_NOTIFY_UPDATE_ERR, NULL, 0);
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
return -1;
} }
*as4_aggregator_as = stream_getl (peer->ibuf); *as4_aggregator_as = stream_getl (peer->ibuf);
as4_aggregator_addr->s_addr = stream_get_ipv4 (peer->ibuf); as4_aggregator_addr->s_addr = stream_get_ipv4 (peer->ibuf);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* Munge Aggregator and New-Aggregator, AS_PATH and NEW_AS_PATH. /* Munge Aggregator and New-Aggregator, AS_PATH and NEW_AS_PATH.
*/ */
static int static bgp_attr_parse_ret_t
bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr, bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr, u_char flag,
struct aspath *as4_path, as_t as4_aggregator, struct aspath *as4_path, as_t as4_aggregator,
struct in_addr *as4_aggregator_addr) struct in_addr *as4_aggregator_addr)
{ {
@ -1050,7 +1133,7 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
struct aspath *newpath; struct aspath *newpath;
struct attr_extra *attre = attr->extra; struct attr_extra *attre = attr->extra;
if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV) ) if (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV))
{ {
/* peer can do AS4, so we ignore AS4_PATH and AS4_AGGREGATOR /* peer can do AS4, so we ignore AS4_PATH and AS4_AGGREGATOR
* if given. * if given.
@ -1068,11 +1151,11 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
peer->host, "AS4 capable peer, yet it sent"); peer->host, "AS4 capable peer, yet it sent");
} }
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
if (attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS4_PATH)) if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH))
&& !(attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS_PATH)))) && !(attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS_PATH))))
{ {
/* Hu? This is not supposed to happen at all! /* Hu? This is not supposed to happen at all!
* got as4_path and no aspath, * got as4_path and no aspath,
@ -1084,10 +1167,9 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
zlog (peer->log, LOG_ERR, zlog (peer->log, LOG_ERR,
"%s BGP not AS4 capable peer sent AS4_PATH but" "%s BGP not AS4 capable peer sent AS4_PATH but"
" no AS_PATH, cant do anything here", peer->host); " no AS_PATH, cant do anything here", peer->host);
bgp_notify_send (peer, return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_MAL_ATTR,
BGP_NOTIFY_UPDATE_MAL_ATTR); NULL, 0);
return -1;
} }
/* We have a asn16 peer. First, look for AS4_AGGREGATOR /* We have a asn16 peer. First, look for AS4_AGGREGATOR
@ -1095,7 +1177,7 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
*/ */
if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR) ) ) if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR) ) )
{ {
if ( attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR) ) ) if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR) ) )
{ {
assert (attre); assert (attre);
@ -1111,7 +1193,7 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
* Aggregating node and the AS_PATH is to be * Aggregating node and the AS_PATH is to be
* constructed "as in all other cases" * constructed "as in all other cases"
*/ */
if ( attre->aggregator_as != BGP_AS_TRANS ) if (attre->aggregator_as != BGP_AS_TRANS)
{ {
/* ignore */ /* ignore */
if ( BGP_DEBUG(as4, AS4)) if ( BGP_DEBUG(as4, AS4))
@ -1146,24 +1228,27 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr,
} }
/* need to reconcile NEW_AS_PATH and AS_PATH */ /* need to reconcile NEW_AS_PATH and AS_PATH */
if ( !ignore_as4_path && (attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS4_PATH))) ) if (!ignore_as4_path && (attr->flag & (ATTR_FLAG_BIT( BGP_ATTR_AS4_PATH))))
{ {
newpath = aspath_reconcile_as4 (attr->aspath, as4_path); newpath = aspath_reconcile_as4 (attr->aspath, as4_path);
aspath_unintern (&attr->aspath); aspath_unintern (&attr->aspath);
attr->aspath = aspath_intern (newpath); attr->aspath = aspath_intern (newpath);
} }
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* Community attribute. */ /* Community attribute. */
static int static bgp_attr_parse_ret_t
bgp_attr_community (struct peer *peer, bgp_size_t length, bgp_attr_community (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag) struct attr *attr, u_char flag, u_char *startp)
{ {
bgp_size_t total
= length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
if (length == 0) if (length == 0)
{ {
attr->community = NULL; attr->community = NULL;
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
attr->community = attr->community =
@ -1173,15 +1258,17 @@ bgp_attr_community (struct peer *peer, bgp_size_t length,
stream_forward_getp (peer->ibuf, length); stream_forward_getp (peer->ibuf, length);
if (!attr->community) if (!attr->community)
return -1; return bgp_attr_malformed (peer, BGP_ATTR_COMMUNITIES, flag,
BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
startp, total);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* Originator ID attribute. */ /* Originator ID attribute. */
static int static bgp_attr_parse_ret_t
bgp_attr_originator_id (struct peer *peer, bgp_size_t length, bgp_attr_originator_id (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag) struct attr *attr, u_char flag)
{ {
@ -1189,10 +1276,9 @@ bgp_attr_originator_id (struct peer *peer, bgp_size_t length,
{ {
zlog (peer->log, LOG_ERR, "Bad originator ID length %d", length); zlog (peer->log, LOG_ERR, "Bad originator ID length %d", length);
bgp_notify_send (peer, return bgp_attr_malformed (peer, BGP_ATTR_ORIGINATOR_ID, flag,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); NULL, 0);
return -1;
} }
(bgp_attr_extra_get (attr))->originator_id.s_addr (bgp_attr_extra_get (attr))->originator_id.s_addr
@ -1200,11 +1286,11 @@ bgp_attr_originator_id (struct peer *peer, bgp_size_t length,
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGINATOR_ID); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGINATOR_ID);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* Cluster list attribute. */ /* Cluster list attribute. */
static int static bgp_attr_parse_ret_t
bgp_attr_cluster_list (struct peer *peer, bgp_size_t length, bgp_attr_cluster_list (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag) struct attr *attr, u_char flag)
{ {
@ -1213,20 +1299,20 @@ bgp_attr_cluster_list (struct peer *peer, bgp_size_t length,
{ {
zlog (peer->log, LOG_ERR, "Bad cluster list length %d", length); zlog (peer->log, LOG_ERR, "Bad cluster list length %d", length);
bgp_notify_send (peer, return bgp_attr_malformed (peer, BGP_ATTR_CLUSTER_LIST, flag,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); NULL, 0);
return -1;
} }
(bgp_attr_extra_get (attr))->cluster (bgp_attr_extra_get (attr))->cluster
= cluster_parse ((struct in_addr *)stream_pnt (peer->ibuf), length); = cluster_parse ((struct in_addr *)stream_pnt (peer->ibuf), length);
stream_forward_getp (peer->ibuf, length);; /* XXX: Fix cluster_parse to use stream API and then remove this */
stream_forward_getp (peer->ibuf, length);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_CLUSTER_LIST); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_CLUSTER_LIST);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* Multiprotocol reachability information parse. */ /* Multiprotocol reachability information parse. */
@ -1253,7 +1339,7 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
{ {
zlog_info ("%s: %s sent invalid length, %lu", zlog_info ("%s: %s sent invalid length, %lu",
__func__, peer->host, (unsigned long)length); __func__, peer->host, (unsigned long)length);
return -1; return BGP_ATTR_PARSE_ERROR;
} }
/* Load AFI, SAFI. */ /* Load AFI, SAFI. */
@ -1267,7 +1353,7 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
{ {
zlog_info ("%s: %s, MP nexthop length, %u, goes past end of attribute", zlog_info ("%s: %s, MP nexthop length, %u, goes past end of attribute",
__func__, peer->host, attre->mp_nexthop_len); __func__, peer->host, attre->mp_nexthop_len);
return -1; return BGP_ATTR_PARSE_ERROR;
} }
/* Nexthop length check. */ /* Nexthop length check. */
@ -1315,14 +1401,14 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
default: default:
zlog_info ("%s: (%s) Wrong multiprotocol next hop length: %d", zlog_info ("%s: (%s) Wrong multiprotocol next hop length: %d",
__func__, peer->host, attre->mp_nexthop_len); __func__, peer->host, attre->mp_nexthop_len);
return -1; return BGP_ATTR_PARSE_ERROR;
} }
if (!LEN_LEFT) if (!LEN_LEFT)
{ {
zlog_info ("%s: (%s) Failed to read SNPA and NLRI(s)", zlog_info ("%s: (%s) Failed to read SNPA and NLRI(s)",
__func__, peer->host); __func__, peer->host);
return -1; return BGP_ATTR_PARSE_ERROR;
} }
{ {
@ -1338,7 +1424,7 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
{ {
zlog_info ("%s: (%s) Failed to read NLRI", zlog_info ("%s: (%s) Failed to read NLRI",
__func__, peer->host); __func__, peer->host);
return -1; return BGP_ATTR_PARSE_ERROR;
} }
if (safi != BGP_SAFI_VPNV4) if (safi != BGP_SAFI_VPNV4)
@ -1348,7 +1434,7 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
{ {
zlog_info ("%s: (%s) NLRI doesn't pass sanity check", zlog_info ("%s: (%s) NLRI doesn't pass sanity check",
__func__, peer->host); __func__, peer->host);
return -1; return BGP_ATTR_PARSE_ERROR;
} }
} }
@ -1359,7 +1445,7 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr,
stream_forward_getp (s, nlri_len); stream_forward_getp (s, nlri_len);
return 0; return BGP_ATTR_PARSE_PROCEED;
#undef LEN_LEFT #undef LEN_LEFT
} }
@ -1378,7 +1464,7 @@ bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length,
#define BGP_MP_UNREACH_MIN_SIZE 3 #define BGP_MP_UNREACH_MIN_SIZE 3
if ((length > STREAM_READABLE(s)) || (length < BGP_MP_UNREACH_MIN_SIZE)) if ((length > STREAM_READABLE(s)) || (length < BGP_MP_UNREACH_MIN_SIZE))
return -1; return BGP_ATTR_PARSE_ERROR;
afi = stream_getw (s); afi = stream_getw (s);
safi = stream_getc (s); safi = stream_getc (s);
@ -1389,7 +1475,7 @@ bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length,
{ {
ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), withdraw_len); ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), withdraw_len);
if (ret < 0) if (ret < 0)
return -1; return BGP_ATTR_PARSE_ERROR;
} }
mp_withdraw->afi = afi; mp_withdraw->afi = afi;
@ -1399,20 +1485,23 @@ bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length,
stream_forward_getp (s, withdraw_len); stream_forward_getp (s, withdraw_len);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* Extended Community attribute. */ /* Extended Community attribute. */
static int static bgp_attr_parse_ret_t
bgp_attr_ext_communities (struct peer *peer, bgp_size_t length, bgp_attr_ext_communities (struct peer *peer, bgp_size_t length,
struct attr *attr, u_char flag) struct attr *attr, u_char flag, u_char *startp)
{ {
bgp_size_t total
= length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
if (length == 0) if (length == 0)
{ {
if (attr->extra) if (attr->extra)
attr->extra->ecommunity = NULL; attr->extra->ecommunity = NULL;
/* Empty extcomm doesn't seem to be invalid per se */ /* Empty extcomm doesn't seem to be invalid per se */
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
(bgp_attr_extra_get (attr))->ecommunity = (bgp_attr_extra_get (attr))->ecommunity =
@ -1421,15 +1510,17 @@ bgp_attr_ext_communities (struct peer *peer, bgp_size_t length,
stream_forward_getp (peer->ibuf, length); stream_forward_getp (peer->ibuf, length);
if (!attr->extra->ecommunity) if (!attr->extra->ecommunity)
return -1; return bgp_attr_malformed (peer, BGP_ATTR_EXT_COMMUNITIES,
flag, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
startp, total);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* BGP unknown attribute treatment. */ /* BGP unknown attribute treatment. */
static int static bgp_attr_parse_ret_t
bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag, bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag,
u_char type, bgp_size_t length, u_char *startp) u_char type, bgp_size_t length, u_char *startp)
{ {
@ -1455,20 +1546,17 @@ bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag,
then the Error Subcode is set to Unrecognized Well-known then the Error Subcode is set to Unrecognized Well-known
Attribute. The Data field contains the unrecognized attribute Attribute. The Data field contains the unrecognized attribute
(type, length and value). */ (type, length and value). */
if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)) if (!CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
{ {
/* Adjust startp to do not include flag value. */ return bgp_attr_malformed (peer, type, flag,
bgp_notify_send_with_data (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_UNREC_ATTR, BGP_NOTIFY_UPDATE_UNREC_ATTR,
startp, total); startp, total);
return -1;
} }
/* Unrecognized non-transitive optional attributes must be quietly /* Unrecognized non-transitive optional attributes must be quietly
ignored and not passed along to other BGP peers. */ ignored and not passed along to other BGP peers. */
if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS)) if (! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
return 0; return BGP_ATTR_PARSE_PROCEED;
/* If a path with recognized transitive optional attribute is /* If a path with recognized transitive optional attribute is
accepted and passed along to other BGP peers and the Partial bit accepted and passed along to other BGP peers and the Partial bit
@ -1491,17 +1579,17 @@ bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag,
memcpy (transit->val + transit->length, startp, total); memcpy (transit->val + transit->length, startp, total);
transit->length += total; transit->length += total;
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* Read attribute of update packet. This function is called from /* Read attribute of update packet. This function is called from
bgp_update() in bgpd.c. */ bgp_update() in bgpd.c. */
int bgp_attr_parse_ret_t
bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
struct bgp_nlri *mp_update, struct bgp_nlri *mp_withdraw) struct bgp_nlri *mp_update, struct bgp_nlri *mp_withdraw)
{ {
int ret; int ret;
u_char flag; u_char flag = 0;
u_char type = 0; u_char type = 0;
bgp_size_t length; bgp_size_t length;
u_char *startp, *endp; u_char *startp, *endp;
@ -1534,7 +1622,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
bgp_notify_send (peer, bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
return -1; return BGP_ATTR_PARSE_ERROR;
} }
/* Fetch attribute flag and type. */ /* Fetch attribute flag and type. */
@ -1554,7 +1642,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
bgp_notify_send (peer, bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
return -1; return BGP_ATTR_PARSE_ERROR;
} }
/* Check extended attribue length bit. */ /* Check extended attribue length bit. */
@ -1576,7 +1664,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
bgp_notify_send (peer, bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_MAL_ATTR); BGP_NOTIFY_UPDATE_MAL_ATTR);
return -1; return BGP_ATTR_PARSE_ERROR;
} }
/* Set type to bitmap to check duplicate attribute. `type' is /* Set type to bitmap to check duplicate attribute. `type' is
@ -1594,7 +1682,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
bgp_notify_send (peer, bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
return -1; return BGP_ATTR_PARSE_ERROR;
} }
/* OK check attribute and store it's value. */ /* OK check attribute and store it's value. */
@ -1607,7 +1695,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
ret = bgp_attr_aspath (peer, length, attr, flag, startp); ret = bgp_attr_aspath (peer, length, attr, flag, startp);
break; break;
case BGP_ATTR_AS4_PATH: case BGP_ATTR_AS4_PATH:
ret = bgp_attr_as4_path (peer, length, attr, &as4_path ); ret = bgp_attr_as4_path (peer, length, attr, flag, startp, &as4_path);
break; break;
case BGP_ATTR_NEXT_HOP: case BGP_ATTR_NEXT_HOP:
ret = bgp_attr_nexthop (peer, length, attr, flag, startp); ret = bgp_attr_nexthop (peer, length, attr, flag, startp);
@ -1625,10 +1713,12 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
ret = bgp_attr_aggregator (peer, length, attr, flag); ret = bgp_attr_aggregator (peer, length, attr, flag);
break; break;
case BGP_ATTR_AS4_AGGREGATOR: case BGP_ATTR_AS4_AGGREGATOR:
ret = bgp_attr_as4_aggregator (peer, length, attr, &as4_aggregator, &as4_aggregator_addr); ret = bgp_attr_as4_aggregator (peer, length, attr, flag,
&as4_aggregator,
&as4_aggregator_addr);
break; break;
case BGP_ATTR_COMMUNITIES: case BGP_ATTR_COMMUNITIES:
ret = bgp_attr_community (peer, length, attr, flag); ret = bgp_attr_community (peer, length, attr, flag, startp);
break; break;
case BGP_ATTR_ORIGINATOR_ID: case BGP_ATTR_ORIGINATOR_ID:
ret = bgp_attr_originator_id (peer, length, attr, flag); ret = bgp_attr_originator_id (peer, length, attr, flag);
@ -1643,15 +1733,15 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
ret = bgp_mp_unreach_parse (peer, length, mp_withdraw); ret = bgp_mp_unreach_parse (peer, length, mp_withdraw);
break; break;
case BGP_ATTR_EXT_COMMUNITIES: case BGP_ATTR_EXT_COMMUNITIES:
ret = bgp_attr_ext_communities (peer, length, attr, flag); ret = bgp_attr_ext_communities (peer, length, attr, flag, startp);
break; break;
default: default:
ret = bgp_attr_unknown (peer, attr, flag, type, length, startp); ret = bgp_attr_unknown (peer, attr, flag, type, length, startp);
break; break;
} }
/* If error occured immediately return to the caller. */ /* If hard error occured immediately return to the caller. */
if (ret < 0) if (ret == BGP_ATTR_PARSE_ERROR)
{ {
zlog (peer->log, LOG_WARNING, zlog (peer->log, LOG_WARNING,
"%s: Attribute %s, parse error", "%s: Attribute %s, parse error",
@ -1660,6 +1750,19 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
bgp_notify_send (peer, bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_MAL_ATTR); BGP_NOTIFY_UPDATE_MAL_ATTR);
if (as4_path)
aspath_unintern (&as4_path);
return ret;
}
if (ret == BGP_ATTR_PARSE_WITHDRAW)
{
zlog (peer->log, LOG_WARNING,
"%s: Attribute %s, parse error - treating as withdrawal",
peer->host,
LOOKUP (attr_str, type));
if (as4_path)
aspath_unintern (&as4_path);
return ret; return ret;
} }
@ -1672,7 +1775,9 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
bgp_notify_send (peer, bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
return -1; if (as4_path)
aspath_unintern (&as4_path);
return BGP_ATTR_PARSE_ERROR;
} }
} }
@ -1685,7 +1790,9 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
bgp_notify_send (peer, bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
return -1; if (as4_path)
aspath_unintern (&as4_path);
return BGP_ATTR_PARSE_ERROR;
} }
/* /*
@ -1699,16 +1806,20 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
* all attributes first, including these 32bit ones, and now, * all attributes first, including these 32bit ones, and now,
* afterwards, we look what and if something is to be done for as4. * afterwards, we look what and if something is to be done for as4.
*/ */
if (bgp_attr_munge_as4_attrs (peer, attr, as4_path, if (bgp_attr_munge_as4_attrs (peer, attr, flag, as4_path,
as4_aggregator, &as4_aggregator_addr)) as4_aggregator, &as4_aggregator_addr))
return -1; {
if (as4_path)
aspath_unintern (&as4_path);
return BGP_ATTR_PARSE_ERROR;
}
/* At this stage, we have done all fiddling with as4, and the /* At this stage, we have done all fiddling with as4, and the
* resulting info is in attr->aggregator resp. attr->aspath * resulting info is in attr->aggregator resp. attr->aspath
* so we can chuck as4_aggregator and as4_path alltogether in * so we can chuck as4_aggregator and as4_path alltogether in
* order to save memory * order to save memory
*/ */
if ( as4_path ) if (as4_path)
{ {
aspath_unintern (&as4_path); /* unintern - it is in the hash */ aspath_unintern (&as4_path); /* unintern - it is in the hash */
/* The flag that we got this is still there, but that does not /* The flag that we got this is still there, but that does not
@ -1725,10 +1836,10 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
* Finally do the checks on the aspath we did not do yet * Finally do the checks on the aspath we did not do yet
* because we waited for a potentially synthesized aspath. * because we waited for a potentially synthesized aspath.
*/ */
if ( attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS_PATH))) if (attr->flag & (ATTR_FLAG_BIT(BGP_ATTR_AS_PATH)))
{ {
ret = bgp_attr_aspath_check( peer, attr ); ret = bgp_attr_aspath_check (peer, attr, flag);
if ( ret < 0 ) if (ret != BGP_ATTR_PARSE_PROCEED)
return ret; return ret;
} }
@ -1736,7 +1847,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
if (attr->extra && attr->extra->transit) if (attr->extra && attr->extra->transit)
attr->extra->transit = transit_intern (attr->extra->transit); attr->extra->transit = transit_intern (attr->extra->transit);
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
/* Well-known attribute check. */ /* Well-known attribute check. */
@ -1767,9 +1878,9 @@ bgp_attr_check (struct peer *peer, struct attr *attr)
BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_MISS_ATTR, BGP_NOTIFY_UPDATE_MISS_ATTR,
&type, 1); &type, 1);
return -1; return BGP_ATTR_PARSE_ERROR;
} }
return 0; return BGP_ATTR_PARSE_PROCEED;
} }
int stream_put_prefix (struct stream *, struct prefix *); int stream_put_prefix (struct stream *, struct prefix *);

12
bgpd/bgp_attr.h
View File

@ -132,16 +132,24 @@ struct transit
#define ATTR_FLAG_BIT(X) (1 << ((X) - 1)) #define ATTR_FLAG_BIT(X) (1 << ((X) - 1))
typedef enum {
BGP_ATTR_PARSE_PROCEED = 0,
BGP_ATTR_PARSE_ERROR = -1,
BGP_ATTR_PARSE_WITHDRAW = -2,
} bgp_attr_parse_ret_t;
/* Prototypes. */ /* Prototypes. */
extern void bgp_attr_init (void); extern void bgp_attr_init (void);
extern void bgp_attr_finish (void); extern void bgp_attr_finish (void);
extern int bgp_attr_parse (struct peer *, struct attr *, bgp_size_t, extern bgp_attr_parse_ret_t bgp_attr_parse (struct peer *, struct attr *,
struct bgp_nlri *, struct bgp_nlri *); bgp_size_t, struct bgp_nlri *,
struct bgp_nlri *);
extern int bgp_attr_check (struct peer *, struct attr *); extern int bgp_attr_check (struct peer *, struct attr *);
extern struct attr_extra *bgp_attr_extra_get (struct attr *); extern struct attr_extra *bgp_attr_extra_get (struct attr *);
extern void bgp_attr_extra_free (struct attr *); extern void bgp_attr_extra_free (struct attr *);
extern void bgp_attr_dup (struct attr *, struct attr *); extern void bgp_attr_dup (struct attr *, struct attr *);
extern struct attr *bgp_attr_intern (struct attr *attr); extern struct attr *bgp_attr_intern (struct attr *attr);
extern void bgp_attr_unintern_sub (struct attr *);
extern void bgp_attr_unintern (struct attr **); extern void bgp_attr_unintern (struct attr **);
extern void bgp_attr_flush (struct attr *); extern void bgp_attr_flush (struct attr *);
extern struct attr *bgp_attr_default_set (struct attr *attr, u_char); extern struct attr *bgp_attr_default_set (struct attr *attr, u_char);

64
bgpd/bgp_packet.c
View File

@ -1590,22 +1590,43 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
return -1; return -1;
} }
/* Certain attribute parsing errors should not be considered bad enough
* to reset the session for, most particularly any partial/optional
* attributes that have 'tunneled' over speakers that don't understand
* them. Instead we withdraw only the prefix concerned.
*
* Complicates the flow a little though..
*/
bgp_attr_parse_ret_t attr_parse_ret = BGP_ATTR_PARSE_PROCEED;
/* This define morphs the update case into a withdraw when lower levels
* have signalled an error condition where this is best.
*/
#define NLRI_ATTR_ARG (attr_parse_ret != BGP_ATTR_PARSE_WITHDRAW ? &attr : NULL)
/* Parse attribute when it exists. */ /* Parse attribute when it exists. */
if (attribute_len) if (attribute_len)
{ {
ret = bgp_attr_parse (peer, &attr, attribute_len, attr_parse_ret = bgp_attr_parse (peer, &attr, attribute_len,
&mp_update, &mp_withdraw); &mp_update, &mp_withdraw);
if (ret < 0) if (attr_parse_ret == BGP_ATTR_PARSE_ERROR)
return -1; return -1;
} }
/* Logging the attribute. */ /* Logging the attribute. */
if (BGP_DEBUG (update, UPDATE_IN)) if (attr_parse_ret == BGP_ATTR_PARSE_WITHDRAW
|| BGP_DEBUG (update, UPDATE_IN))
{ {
ret= bgp_dump_attr (peer, &attr, attrstr, BUFSIZ); ret= bgp_dump_attr (peer, &attr, attrstr, BUFSIZ);
int lvl = (attr_parse_ret == BGP_ATTR_PARSE_WITHDRAW)
? LOG_ERR : LOG_DEBUG;
if (attr_parse_ret == BGP_ATTR_PARSE_WITHDRAW)
zlog (peer->log, LOG_ERR,
"%s rcvd UPDATE with errors in attr(s)!! Withdrawing route.",
peer->host);
if (ret) if (ret)
zlog (peer->log, LOG_DEBUG, "%s rcvd UPDATE w/ attr: %s", zlog (peer->log, lvl, "%s rcvd UPDATE w/ attr: %s",
peer->host, attrstr); peer->host, attrstr);
} }
@ -1617,7 +1638,12 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
/* Check NLRI packet format and prefix length. */ /* Check NLRI packet format and prefix length. */
ret = bgp_nlri_sanity_check (peer, AFI_IP, stream_pnt (s), update_len); ret = bgp_nlri_sanity_check (peer, AFI_IP, stream_pnt (s), update_len);
if (ret < 0) if (ret < 0)
{
bgp_attr_unintern_sub (&attr);
if (attr.extra)
bgp_attr_extra_free (&attr);
return -1; return -1;
}
/* Set NLRI portion to structure. */ /* Set NLRI portion to structure. */
update.afi = AFI_IP; update.afi = AFI_IP;
@ -1640,15 +1666,20 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
update. */ update. */
ret = bgp_attr_check (peer, &attr); ret = bgp_attr_check (peer, &attr);
if (ret < 0) if (ret < 0)
{
bgp_attr_unintern_sub (&attr);
if (attr.extra)
bgp_attr_extra_free (&attr);
return -1; return -1;
}
bgp_nlri_parse (peer, &attr, &update); bgp_nlri_parse (peer, NLRI_ATTR_ARG, &update);
} }
if (mp_update.length if (mp_update.length
&& mp_update.afi == AFI_IP && mp_update.afi == AFI_IP
&& mp_update.safi == SAFI_UNICAST) && mp_update.safi == SAFI_UNICAST)
bgp_nlri_parse (peer, &attr, &mp_update); bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update);
if (mp_withdraw.length if (mp_withdraw.length
&& mp_withdraw.afi == AFI_IP && mp_withdraw.afi == AFI_IP
@ -1675,7 +1706,7 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
if (mp_update.length if (mp_update.length
&& mp_update.afi == AFI_IP && mp_update.afi == AFI_IP
&& mp_update.safi == SAFI_MULTICAST) && mp_update.safi == SAFI_MULTICAST)
bgp_nlri_parse (peer, &attr, &mp_update); bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update);
if (mp_withdraw.length if (mp_withdraw.length
&& mp_withdraw.afi == AFI_IP && mp_withdraw.afi == AFI_IP
@ -1705,7 +1736,7 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
if (mp_update.length if (mp_update.length
&& mp_update.afi == AFI_IP6 && mp_update.afi == AFI_IP6
&& mp_update.safi == SAFI_UNICAST) && mp_update.safi == SAFI_UNICAST)
bgp_nlri_parse (peer, &attr, &mp_update); bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update);
if (mp_withdraw.length if (mp_withdraw.length
&& mp_withdraw.afi == AFI_IP6 && mp_withdraw.afi == AFI_IP6
@ -1734,7 +1765,7 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
if (mp_update.length if (mp_update.length
&& mp_update.afi == AFI_IP6 && mp_update.afi == AFI_IP6
&& mp_update.safi == SAFI_MULTICAST) && mp_update.safi == SAFI_MULTICAST)
bgp_nlri_parse (peer, &attr, &mp_update); bgp_nlri_parse (peer, NLRI_ATTR_ARG, &mp_update);
if (mp_withdraw.length if (mp_withdraw.length
&& mp_withdraw.afi == AFI_IP6 && mp_withdraw.afi == AFI_IP6
@ -1762,7 +1793,7 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
if (mp_update.length if (mp_update.length
&& mp_update.afi == AFI_IP && mp_update.afi == AFI_IP
&& mp_update.safi == BGP_SAFI_VPNV4) && mp_update.safi == BGP_SAFI_VPNV4)
bgp_nlri_parse_vpnv4 (peer, &attr, &mp_update); bgp_nlri_parse_vpnv4 (peer, NLRI_ATTR_ARG, &mp_update);
if (mp_withdraw.length if (mp_withdraw.length
&& mp_withdraw.afi == AFI_IP && mp_withdraw.afi == AFI_IP
@ -1784,20 +1815,9 @@ bgp_update_receive (struct peer *peer, bgp_size_t size)
/* Everything is done. We unintern temporary structures which /* Everything is done. We unintern temporary structures which
interned in bgp_attr_parse(). */ interned in bgp_attr_parse(). */
if (attr.aspath) bgp_attr_unintern_sub (&attr);
aspath_unintern (&attr.aspath);
if (attr.community)
community_unintern (&attr.community);
if (attr.extra) if (attr.extra)
{
if (attr.extra->ecommunity)
ecommunity_unintern (&attr.extra->ecommunity);
if (attr.extra->cluster)
cluster_unintern (attr.extra->cluster);
if (attr.extra->transit)
transit_unintern (attr.extra->transit);
bgp_attr_extra_free (&attr); bgp_attr_extra_free (&attr);
}
/* If peering is stopped due to some reason, do not generate BGP /* If peering is stopped due to some reason, do not generate BGP
event. */ event. */

15
tests/aspath_test.c
View File

@ -408,7 +408,7 @@ static struct test_segment {
"#ASNs = 0, data = seq(8466 3 52737 4096 3456)", "#ASNs = 0, data = seq(8466 3 52737 4096 3456)",
{ 0x2,0x0, 0x21,0x12, 0x00,0x03, 0xce,0x01, 0x10,0x00, 0x0d,0x80 }, { 0x2,0x0, 0x21,0x12, 0x00,0x03, 0xce,0x01, 0x10,0x00, 0x0d,0x80 },
12, 12,
{ "", "", { NULL, NULL,
0, 0, 0, 0, 0, 0 }, 0, 0, 0, 0, 0, 0 },
}, },
{ /* 26 */ { /* 26 */
@ -418,7 +418,7 @@ static struct test_segment {
0x2,0x2, 0x10,0x00, 0x0d,0x80 }, 0x2,0x2, 0x10,0x00, 0x0d,0x80 },
14 14
, ,
{ "", "", { NULL, NULL,
0, 0, 0, 0, 0, 0 }, 0, 0, 0, 0, 0, 0 },
}, },
{ /* 27 */ { /* 27 */
@ -427,7 +427,7 @@ static struct test_segment {
{ 0x8,0x2, 0x10,0x00, 0x0d,0x80 }, { 0x8,0x2, 0x10,0x00, 0x0d,0x80 },
14 14
, ,
{ "", "", { NULL, NULL,
0, 0, 0, 0, 0, 0 }, 0, 0, 0, 0, 0, 0 },
}, { NULL, NULL, {0}, 0, { NULL, 0, 0 } } }, { NULL, NULL, {0}, 0, { NULL, 0, 0 } }
}; };
@ -869,6 +869,12 @@ validate (struct aspath *as, const struct test_spec *sp)
static struct stream *s; static struct stream *s;
struct aspath *asinout, *asconfeddel, *asstr, *as4; struct aspath *asinout, *asconfeddel, *asstr, *as4;
if (as == NULL && sp->shouldbe == NULL)
{
printf ("Correctly failed to parse\n");
return fails;
}
out = aspath_snmp_pathseg (as, &bytes); out = aspath_snmp_pathseg (as, &bytes);
asinout = make_aspath (out, bytes, 0); asinout = make_aspath (out, bytes, 0);
@ -1022,6 +1028,8 @@ parse_test (struct test_segment *t)
printf (FAILED "\n"); printf (FAILED "\n");
printf ("\n"); printf ("\n");
if (asp)
aspath_unintern (asp); aspath_unintern (asp);
} }
@ -1078,6 +1086,7 @@ empty_prepend_test (struct test_segment *t)
printf (FAILED "!\n"); printf (FAILED "!\n");
printf ("\n"); printf ("\n");
if (asp1)
aspath_unintern (asp1); aspath_unintern (asp1);
aspath_free (asp2); aspath_free (asp2);
} }