bgpd: well-known attr check only run for v4/uni, which could cause a crash.
* ANVL testing by Martin Winter threw up a crash in bgpd in aspath_dup
called from bgp_packet_attribute, if attr->aspath was NULL, on an IPv6
UPDATE.
This root cause is that the checks for well-known, mandatory attributes
were being applied only if an UPDATE contained the IPv4 NLRI and the
peer was configured for v4/unicast (i.e. not deconfigured). This is
something inherited from GNU Zebra, and never noticed before.
* bgp_attr.c: (bgp_attr_parse) Move the well-known mandatory attribute
check to here, so that it can be run immediately after all attributes
are parsed, and before any further processing of attributes that might
assume the existence of WK/M attributes (e.g. AS4-Path).
(bgp_attr_munge_as4_attrs) Missing AS_PATH shouldn't happen here anymore,
but retain a check anyway for robustness - it's definitely a hard error
though.
* bgp_attr.h: (bgp_attr_check) No longer needs to be exported, make static.
* bgp_packet.c: (bgp_update_receive) Responsibility for well-known check
now in bgp_attr_parse.
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index b62a4f8..69ca786 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -1282,7 +1282,17 @@
int ignore_as4_path = 0;
struct aspath *newpath;
struct attr_extra *attre = attr->extra;
-
+
+ if (!attr->aspath)
+ {
+ /* NULL aspath shouldn't be possible as bgp_attr_parse should have
+ * checked that all well-known, mandatory attributes were present.
+ *
+ * Can only be a problem with peer itself - hard error
+ */
+ return BGP_ATTR_PARSE_ERROR;
+ }
+
if (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV))
{
/* peer can do AS4, so we ignore AS4_PATH and AS4_AGGREGATOR
@@ -1362,12 +1372,9 @@
/* need to reconcile NEW_AS_PATH and AS_PATH */
if (!ignore_as4_path && (attr->flag & (ATTR_FLAG_BIT( BGP_ATTR_AS4_PATH))))
{
- if (!attr->aspath)
- return BGP_ATTR_PARSE_PROCEED;
-
- newpath = aspath_reconcile_as4 (attr->aspath, as4_path);
- aspath_unintern (&attr->aspath);
- attr->aspath = aspath_intern (newpath);
+ newpath = aspath_reconcile_as4 (attr->aspath, as4_path);
+ aspath_unintern (&attr->aspath);
+ attr->aspath = aspath_intern (newpath);
}
return BGP_ATTR_PARSE_PROCEED;
}
@@ -1726,6 +1733,39 @@
return BGP_ATTR_PARSE_PROCEED;
}
+/* Well-known attribute check. */
+static int
+bgp_attr_check (struct peer *peer, struct attr *attr)
+{
+ u_char type = 0;
+
+ if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_ORIGIN)))
+ type = BGP_ATTR_ORIGIN;
+
+ if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_AS_PATH)))
+ type = BGP_ATTR_AS_PATH;
+
+ if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP)))
+ type = BGP_ATTR_NEXT_HOP;
+
+ if (peer->sort == BGP_PEER_IBGP
+ && ! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF)))
+ type = BGP_ATTR_LOCAL_PREF;
+
+ if (type)
+ {
+ zlog (peer->log, LOG_WARNING,
+ "%s Missing well-known attribute %d.",
+ peer->host, type);
+ bgp_notify_send_with_data (peer,
+ BGP_NOTIFY_UPDATE_ERR,
+ BGP_NOTIFY_UPDATE_MISS_ATTR,
+ &type, 1);
+ return BGP_ATTR_PARSE_ERROR;
+ }
+ return BGP_ATTR_PARSE_PROCEED;
+}
+
/* Read attribute of update packet. This function is called from
bgp_update_receive() in bgp_packet.c. */
bgp_attr_parse_ret_t
@@ -1958,7 +1998,6 @@
return BGP_ATTR_PARSE_ERROR;
}
}
-
/* Check final read pointer is same as end pointer. */
if (BGP_INPUT_PNT (peer) != endp)
{
@@ -1972,7 +2011,18 @@
aspath_unintern (&as4_path);
return BGP_ATTR_PARSE_ERROR;
}
-
+
+ /* Check all mandatory well-known attributes are present */
+ {
+ bgp_attr_parse_ret_t ret;
+ if ((ret = bgp_attr_check (peer, attr)) < 0)
+ {
+ if (as4_path)
+ aspath_unintern (&as4_path);
+ return ret;
+ }
+ }
+
/*
* At this place we can see whether we got AS4_PATH and/or
* AS4_AGGREGATOR from a 16Bit peer and act accordingly.
@@ -2033,39 +2083,6 @@
return BGP_ATTR_PARSE_PROCEED;
}
-/* Well-known attribute check. */
-int
-bgp_attr_check (struct peer *peer, struct attr *attr)
-{
- u_char type = 0;
-
- if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_ORIGIN)))
- type = BGP_ATTR_ORIGIN;
-
- if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_AS_PATH)))
- type = BGP_ATTR_AS_PATH;
-
- if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP)))
- type = BGP_ATTR_NEXT_HOP;
-
- if (peer->sort == BGP_PEER_IBGP
- && ! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF)))
- type = BGP_ATTR_LOCAL_PREF;
-
- if (type)
- {
- zlog (peer->log, LOG_WARNING,
- "%s Missing well-known attribute %d.",
- peer->host, type);
- bgp_notify_send_with_data (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_MISS_ATTR,
- &type, 1);
- return BGP_ATTR_PARSE_ERROR;
- }
- return BGP_ATTR_PARSE_PROCEED;
-}
-
int stream_put_prefix (struct stream *, struct prefix *);
size_t
diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index cb401e7..b59fa8e 100644
--- a/bgpd/bgp_attr.h
+++ b/bgpd/bgp_attr.h
@@ -147,7 +147,6 @@
extern bgp_attr_parse_ret_t bgp_attr_parse (struct peer *, struct attr *,
bgp_size_t, struct bgp_nlri *,
struct bgp_nlri *);
-extern int bgp_attr_check (struct peer *, struct attr *);
extern struct attr_extra *bgp_attr_extra_get (struct attr *);
extern void bgp_attr_extra_free (struct attr *);
extern void bgp_attr_dup (struct attr *, struct attr *);
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 0fab1b0..35a22c1 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -1775,18 +1775,7 @@
bgp_nlri_parse (peer, NULL, &withdraw);
if (update.length)
- {
- /* We check well-known attribute only for IPv4 unicast
- update. */
- ret = bgp_attr_check (peer, &attr);
- if (ret < 0)
- {
- bgp_attr_unintern_sub (&attr);
- return -1;
- }
-
bgp_nlri_parse (peer, NLRI_ATTR_ARG, &update);
- }
if (mp_update.length
&& mp_update.afi == AFI_IP