From 2d0652dd1088395c50df8fe1a99f1111b44688c6 Mon Sep 17 00:00:00 2001 From: Ondrej Zajicek Date: Tue, 20 Feb 2024 17:39:05 +0100 Subject: [PATCH] BGP: Maintain valid route attribute flags even in local tables BGP route attributes have flags (Optional, Transitive) that are validated on decode and set to valid value on export. But if such attribute is modified by filter or set internally by BGP during import, then its flags would be zero in local tables. That usually does not matter, as they are not used locally and they were fixed on export, but invalid flags leaked in BMP and MRT dumps. Keep route attribute flags set to valid values even when set by filters or modified by BGP. --- proto/bgp/attrs.c | 12 +++--------- proto/bgp/config.Y | 28 ++++++++++++++-------------- 2 files changed, 17 insertions(+), 23 deletions(-) diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index 4346cd5d..85646647 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -89,7 +89,7 @@ bgp_set_attr(ea_list **attrs, struct linpool *pool, uint code, uint flags, uintp attrs, pool, EA_CODE(PROTOCOL_BGP, code), - flags & ~BAF_EXT_LEN, + bgp_attr_table[code].flags | (flags & BAF_PARTIAL), bgp_attr_table[code].type, val ); @@ -1158,13 +1158,6 @@ bgp_attr_name(uint code) return (code < ARRAY_SIZE(bgp_attr_table)) ? bgp_attr_table[code].name : NULL; } -void bgp_fix_attr_flags(ea_list *attrs) -{ - for (u8 i = 0; i < attrs->count; i++) - { - attrs->attrs[i].flags = bgp_attr_table[EA_ID(attrs->attrs[i].id)].flags; - } -} /* * Attribute export @@ -1182,7 +1175,8 @@ bgp_export_attr(struct bgp_export_state *s, eattr *a, ea_list *to) { const struct bgp_attr_desc *desc = &bgp_attr_table[code]; - /* The flags might have been zero if the attr was added by filters */ + /* The flags should be correct, we reset them just to be sure */ + ASSERT(!((a->flags ^ desc->flags) & (BAF_OPTIONAL | BAF_TRANSITIVE))); a->flags = (a->flags & BAF_PARTIAL) | desc->flags; /* Set partial bit if new opt-trans attribute is attached to non-local route */ diff --git a/proto/bgp/config.Y b/proto/bgp/config.Y index 1173ff06..4ce06488 100644 --- a/proto/bgp/config.Y +++ b/proto/bgp/config.Y @@ -349,33 +349,33 @@ bgp_proto_channel: bgp_channel_start bgp_channel_opt_list bgp_channel_end; dynamic_attr: BGP_ORIGIN - { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_ENUM_BGP_ORIGIN, EA_CODE(PROTOCOL_BGP, BA_ORIGIN)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_ENUM_BGP_ORIGIN, EA_CODE(PROTOCOL_BGP, BA_ORIGIN)); $$.flags = BAF_TRANSITIVE; } ; dynamic_attr: BGP_PATH - { $$ = f_new_dynamic_attr(EAF_TYPE_AS_PATH, T_PATH, EA_CODE(PROTOCOL_BGP, BA_AS_PATH)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_AS_PATH, T_PATH, EA_CODE(PROTOCOL_BGP, BA_AS_PATH)); $$.flags = BAF_TRANSITIVE; } ; dynamic_attr: BGP_NEXT_HOP - { $$ = f_new_dynamic_attr(EAF_TYPE_IP_ADDRESS, T_IP, EA_CODE(PROTOCOL_BGP, BA_NEXT_HOP)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_IP_ADDRESS, T_IP, EA_CODE(PROTOCOL_BGP, BA_NEXT_HOP)); $$.flags = BAF_TRANSITIVE; } ; dynamic_attr: BGP_MED - { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(PROTOCOL_BGP, BA_MULTI_EXIT_DISC)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(PROTOCOL_BGP, BA_MULTI_EXIT_DISC)); $$.flags = BAF_OPTIONAL; } ; dynamic_attr: BGP_LOCAL_PREF - { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(PROTOCOL_BGP, BA_LOCAL_PREF)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(PROTOCOL_BGP, BA_LOCAL_PREF)); $$.flags = BAF_TRANSITIVE; } ; dynamic_attr: BGP_ATOMIC_AGGR - { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_ENUM_EMPTY, EA_CODE(PROTOCOL_BGP, BA_ATOMIC_AGGR)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_ENUM_EMPTY, EA_CODE(PROTOCOL_BGP, BA_ATOMIC_AGGR)); $$.flags = BAF_TRANSITIVE; } ; dynamic_attr: BGP_AGGREGATOR - { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_ENUM_EMPTY, EA_CODE(PROTOCOL_BGP, BA_AGGREGATOR)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_ENUM_EMPTY, EA_CODE(PROTOCOL_BGP, BA_AGGREGATOR)); $$.flags = BAF_OPTIONAL | BAF_TRANSITIVE; } ; dynamic_attr: BGP_COMMUNITY - { $$ = f_new_dynamic_attr(EAF_TYPE_INT_SET, T_CLIST, EA_CODE(PROTOCOL_BGP, BA_COMMUNITY)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_INT_SET, T_CLIST, EA_CODE(PROTOCOL_BGP, BA_COMMUNITY)); $$.flags = BAF_OPTIONAL | BAF_TRANSITIVE; } ; dynamic_attr: BGP_ORIGINATOR_ID - { $$ = f_new_dynamic_attr(EAF_TYPE_ROUTER_ID, T_QUAD, EA_CODE(PROTOCOL_BGP, BA_ORIGINATOR_ID)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_ROUTER_ID, T_QUAD, EA_CODE(PROTOCOL_BGP, BA_ORIGINATOR_ID)); $$.flags = BAF_OPTIONAL; } ; dynamic_attr: BGP_CLUSTER_LIST - { $$ = f_new_dynamic_attr(EAF_TYPE_INT_SET, T_CLIST, EA_CODE(PROTOCOL_BGP, BA_CLUSTER_LIST)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_INT_SET, T_CLIST, EA_CODE(PROTOCOL_BGP, BA_CLUSTER_LIST)); $$.flags = BAF_OPTIONAL; } ; dynamic_attr: BGP_EXT_COMMUNITY - { $$ = f_new_dynamic_attr(EAF_TYPE_EC_SET, T_ECLIST, EA_CODE(PROTOCOL_BGP, BA_EXT_COMMUNITY)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_EC_SET, T_ECLIST, EA_CODE(PROTOCOL_BGP, BA_EXT_COMMUNITY)); $$.flags = BAF_OPTIONAL | BAF_TRANSITIVE; } ; dynamic_attr: BGP_AIGP - { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_ENUM_EMPTY, EA_CODE(PROTOCOL_BGP, BA_AIGP)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_OPAQUE, T_ENUM_EMPTY, EA_CODE(PROTOCOL_BGP, BA_AIGP)); $$.flags = BAF_OPTIONAL; } ; dynamic_attr: BGP_LARGE_COMMUNITY - { $$ = f_new_dynamic_attr(EAF_TYPE_LC_SET, T_LCLIST, EA_CODE(PROTOCOL_BGP, BA_LARGE_COMMUNITY)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_LC_SET, T_LCLIST, EA_CODE(PROTOCOL_BGP, BA_LARGE_COMMUNITY)); $$.flags = BAF_OPTIONAL | BAF_TRANSITIVE; } ; dynamic_attr: BGP_OTC - { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(PROTOCOL_BGP, BA_ONLY_TO_CUSTOMER)); } ; + { $$ = f_new_dynamic_attr(EAF_TYPE_INT, T_INT, EA_CODE(PROTOCOL_BGP, BA_ONLY_TO_CUSTOMER)); $$.flags = BAF_OPTIONAL | BAF_TRANSITIVE; } ; custom_attr: ATTRIBUTE BGP expr type symbol ';' { if ($3 > 255 || $3 < 1)