bgpd: OPEN parse errors should send OPEN_ERR and UNSPECIFIC subcode.
CEASE NOTIFICATION for OPEN parsing errors seems, to my reading of RFC4271
ยง6.2 to be incorrect.
* bgp_packet.c: (bgp_open_receive) OPEN/UNACEP_HOLDTIME is not an
appropriate error subcode if bgp_open_option_parse returns an error. Set
it to "Unspecific". Where a more specific subcode is appropriate, then lower
level should send that.
* bgp_open.c: (bgp_open_option_parse) Malformed OPENs should result in
NOTIFICATION with OPEN error, and OPEN/UNSPECIFIC sub-code - not CEASE.
(bgp_capability_{parse,orf_entry}) ditto.
* bgpd.h: Add BGP_NOTIFY_OPEN_UNSPECIFIC for 0. Note that IANA lists 0 as
reserved in the OPEN error sub-code registry, but RFC4271 page 32 says 0
is the "Unspecific" OPEN error subcode.
Have emailed IANA, they says it's a known errate to 4271 under review.
Some inspiration from Cumulus' bgpd-capability-cleanup.patch, though
v different result.
diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c
index b9d6e93..7273882 100644
--- a/bgpd/bgp_open.c
+++ b/bgpd/bgp_open.c
@@ -235,7 +235,7 @@
zlog_info ("%s ORF Capability entry length error,"
" Cap length %u, num %u",
peer->host, hdr->length, entry.num);
- bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
+ bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_UNSPECIFIC);
return -1;
}
@@ -469,7 +469,7 @@
if (stream_get_getp(s) + 2 > end)
{
zlog_info ("%s Capability length error (< header)", peer->host);
- bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
+ bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_UNSPECIFIC);
return -1;
}
@@ -481,7 +481,7 @@
if (start + caphdr.length > end)
{
zlog_info ("%s Capability length error (< length)", peer->host);
- bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
+ bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR, BGP_NOTIFY_OPEN_UNSPECIFIC);
return -1;
}
@@ -511,7 +511,8 @@
LOOKUP (capcode_str, caphdr.code),
caphdr.length,
(unsigned) cap_minsizes[caphdr.code]);
- bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
+ bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR,
+ BGP_NOTIFY_OPEN_UNSPECIFIC);
return -1;
}
/* we deliberately ignore unknown codes, see below */
@@ -727,7 +728,8 @@
if (STREAM_READABLE(s) < 2)
{
zlog_info ("%s Option length error", peer->host);
- bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
+ bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR,
+ BGP_NOTIFY_OPEN_UNSPECIFIC);
return -1;
}
@@ -739,7 +741,8 @@
if (STREAM_READABLE (s) < opt_length)
{
zlog_info ("%s Option length error", peer->host);
- bgp_notify_send (peer, BGP_NOTIFY_CEASE, 0);
+ bgp_notify_send (peer, BGP_NOTIFY_OPEN_ERR,
+ BGP_NOTIFY_OPEN_UNSPECIFIC);
return -1;
}
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index a90e56b..0639f4d 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -1545,7 +1545,7 @@
{
bgp_notify_send (peer,
BGP_NOTIFY_OPEN_ERR,
- BGP_NOTIFY_OPEN_UNACEP_HOLDTIME);
+ BGP_NOTIFY_OPEN_UNSPECIFIC);
return ret;
}
}
diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h
index d4c8dbd..2c4fb20 100644
--- a/bgpd/bgpd.h
+++ b/bgpd/bgpd.h
@@ -672,6 +672,7 @@
#define BGP_NOTIFY_HEADER_MAX 4
/* BGP_NOTIFY_OPEN_ERR sub codes. */
+#define BGP_NOTIFY_OPEN_UNSPECIFIC 0
#define BGP_NOTIFY_OPEN_UNSUP_VERSION 1
#define BGP_NOTIFY_OPEN_BAD_PEER_AS 2
#define BGP_NOTIFY_OPEN_BAD_BGP_IDENT 3