From 65ba595f63ea191b09693c662ba1f5ad23af7597 Mon Sep 17 00:00:00 2001 From: claudio Date: Wed, 5 Sep 2018 07:31:29 +0000 Subject: [PATCH] Change verification of communities a bit. Move the flag checking first since it is currently terminal and then do the length check. If that one triggers do a treat-as-withdraw but at the same time drop the bad attribute since it is better to not have invalid attributes in the Adj-RIB-In since most code does not expect that. OK benno@ --- usr.sbin/bgpd/rde.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c index 207281f71a0..a9a33451bb4 100644 --- a/usr.sbin/bgpd/rde.c +++ b/usr.sbin/bgpd/rde.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rde.c,v 1.416 2018/08/29 19:47:47 claudio Exp $ */ +/* $OpenBSD: rde.c,v 1.417 2018/09/05 07:31:29 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -1583,6 +1583,9 @@ bad_flags: /* 4-byte ready server take the default route */ goto optattr; case ATTR_COMMUNITIES: + if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE, + ATTR_PARTIAL)) + goto bad_flags; if (attr_len == 0 || attr_len % 4 != 0) { /* * mark update as bad and withdraw all routes as per @@ -1591,12 +1594,13 @@ bad_flags: a->flags |= F_ATTR_PARSE_ERR; log_peer_warnx(&peer->conf, "bad COMMUNITIES, " "path invalidated and prefix withdrawn"); + break; } + goto optattr; + case ATTR_LARGE_COMMUNITIES: if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE, ATTR_PARTIAL)) goto bad_flags; - goto optattr; - case ATTR_LARGE_COMMUNITIES: if (attr_len == 0 || attr_len % 12 != 0) { /* * mark update as bad and withdraw all routes as per @@ -1605,12 +1609,13 @@ bad_flags: a->flags |= F_ATTR_PARSE_ERR; log_peer_warnx(&peer->conf, "bad LARGE COMMUNITIES, " "path invalidated and prefix withdrawn"); + break; } + goto optattr; + case ATTR_EXT_COMMUNITIES: if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE, ATTR_PARTIAL)) goto bad_flags; - goto optattr; - case ATTR_EXT_COMMUNITIES: if (attr_len == 0 || attr_len % 8 != 0) { /* * mark update as bad and withdraw all routes as per @@ -1619,10 +1624,8 @@ bad_flags: a->flags |= F_ATTR_PARSE_ERR; log_peer_warnx(&peer->conf, "bad EXT_COMMUNITIES, " "path invalidated and prefix withdrawn"); + break; } - if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE, - ATTR_PARTIAL)) - goto bad_flags; goto optattr; case ATTR_ORIGINATOR_ID: if (attr_len != 4) -- 2.20.1