From 38195ac628797cad5453e91cd6b38bb0b6f34f84 Mon Sep 17 00:00:00 2001 From: Maria Matejka Date: Wed, 6 Nov 2024 12:12:37 +0100 Subject: [PATCH] ASPA: fixed the check algorithm to actually do what is in the RFC The original algorithm assumed principles not consistent with the RFC and could have lead to false invalids. Also added filter tests showing also how the ASPA literals are used in the static protocol. --- filter/config.Y | 2 +- filter/f-inst.c | 7 +-- filter/test.conf | 49 ++++++++++++++++++++ lib/net.h | 3 -- nest/config.Y | 2 +- nest/route.h | 13 ++++-- nest/rt-table.c | 112 ++++++++++++++++++++++++++++++--------------- proto/bgp/config.Y | 27 ++++++++++- 8 files changed, 163 insertions(+), 52 deletions(-) diff --git a/filter/config.Y b/filter/config.Y index 3b4d669f..72996850 100644 --- a/filter/config.Y +++ b/filter/config.Y @@ -951,7 +951,7 @@ term: | ROA_CHECK '(' rtable ')' { $$ = f_new_inst(FI_ROA_CHECK_IMPLICIT, $3); } | ROA_CHECK '(' rtable ',' term ',' term ')' { $$ = f_new_inst(FI_ROA_CHECK_EXPLICIT, $5, $7, $3); } - | ASPA_CHECK '(' rtable ',' term ')' { $$ = f_new_inst(FI_ASPA_CHECK_EXPLICIT, $5, $3); } + | ASPA_CHECK '(' rtable ',' term ',' term ')' { $$ = f_new_inst(FI_ASPA_CHECK_EXPLICIT, $5, $7, $3); } | FORMAT '(' term ')' { $$ = f_new_inst(FI_FORMAT, $3); } diff --git a/filter/f-inst.c b/filter/f-inst.c index 455dd9bb..8529ad62 100644 --- a/filter/f-inst.c +++ b/filter/f-inst.c @@ -1606,10 +1606,11 @@ } - INST(FI_ASPA_CHECK_EXPLICIT, 1, 1) { /* ASPA Check */ + INST(FI_ASPA_CHECK_EXPLICIT, 2, 1) { /* ASPA Check */ NEVER_CONSTANT; ARG(1, T_PATH); - RTC(2); + ARG(2, T_BOOL); + RTC(3); struct rtable *table = rtc->table; if (!table) @@ -1618,7 +1619,7 @@ if (table->addr_type != NET_ASPA) runtime("Table type must be ASPA"); - RESULT(T_ENUM_ASPA, i, [[ aspa_check(table, v1.val.ad) ]]); + RESULT(T_ENUM_ASPA, i, [[ aspa_check(table, v1.val.ad, v2.val.i) ]]); } INST(FI_FROM_HEX, 1, 1) { /* Convert hex text to bytestring */ diff --git a/filter/test.conf b/filter/test.conf index 37f93516..488e2b75 100644 --- a/filter/test.conf +++ b/filter/test.conf @@ -2242,7 +2242,56 @@ prefix pfx; bt_test_suite(t_roa_check, "Testing ROA"); +aspa table at; +protocol static +{ + aspa { table at; }; + route aspa 65540 provider 65544; + route aspa 65541 provider 65545; + route aspa 65542 provider 65544, 65545; + route aspa 65543 provider 65544, 65545; + route aspa 65544 transit; + route aspa 65545 transit; + route aspa 65550 provider 65540; + route aspa 65551 provider 65543; +} + +function t_aspa_check() +{ + bgppath p1 = +empty+; + p1.prepend(65540); + p1.prepend(65544); + bt_assert(aspa_check(at, p1, false) = ASPA_VALID); + bt_assert(aspa_check(at, p1, true) = ASPA_VALID); + + p1.prepend(65542); + bt_assert(aspa_check(at, p1, false) = ASPA_VALID); + bt_assert(aspa_check(at, p1, true) = ASPA_INVALID_LEAK); + + p1.prepend(65555); + bt_assert(aspa_check(at, p1, false) = ASPA_UNKNOWN); + bt_assert(aspa_check(at, p1, true) = ASPA_INVALID_LEAK); + + bgppath p2 = +empty+; + p2.prepend(65554); + p2.prepend(65541); + p2.prepend(65545); + bt_assert(aspa_check(at, p2, false) = ASPA_UNKNOWN); + bt_assert(aspa_check(at, p2, true) = ASPA_UNKNOWN); + + p2.prepend(65543); + bt_assert(aspa_check(at, p2, false) = ASPA_UNKNOWN); + bt_assert(aspa_check(at, p2, true) = ASPA_INVALID_LEAK); + + bgppath p3 = +empty+; + p3.prepend(65541); + p3.prepend(65544); + bt_assert(aspa_check(at, p3, false) = ASPA_INVALID_LEAK); + bt_assert(aspa_check(at, p3, true) = ASPA_INVALID_LEAK); +} + +bt_test_suite(t_aspa_check, "Testing ASPA"); filter vpn_filter { diff --git a/lib/net.h b/lib/net.h index d1b55c07..61ed37ba 100644 --- a/lib/net.h +++ b/lib/net.h @@ -196,9 +196,6 @@ extern const u16 net_max_text_length[]; #define NET_ADDR_ASPA(asn) \ ((net_addr_aspa) { NET_ASPA, 32, sizeof(net_addr_aspa), asn }) -#define NET_ADDR_ASPA_EXISTS(asn) NET_ADDR_ASPA(asn, asn) -#define NET_ADDR_ASPA_TRANSIT(asn) NET_ADDR_ASPA(asn, 0) - #define NET_ADDR_MPLS(label) \ ((net_addr_mpls) { NET_MPLS, 20, sizeof(net_addr_mpls), label }) diff --git a/nest/config.Y b/nest/config.Y index 05f5799f..2b92ea52 100644 --- a/nest/config.Y +++ b/nest/config.Y @@ -139,7 +139,7 @@ CF_ENUM(T_ENUM_RTS, RTS_, STATIC, INHERIT, DEVICE, STATIC_DEVICE, REDIRECT, CF_ENUM(T_ENUM_SCOPE, SCOPE_, HOST, LINK, SITE, ORGANIZATION, UNIVERSE, UNDEFINED) CF_ENUM(T_ENUM_RTD, RTD_, UNICAST, BLACKHOLE, UNREACHABLE, PROHIBIT) CF_ENUM(T_ENUM_ROA, ROA_, UNKNOWN, VALID, INVALID) -CF_ENUM(T_ENUM_ASPA, ASPA_, UNKNOWN, VALID, INVALID) +CF_ENUM(T_ENUM_ASPA, ASPA_, UNKNOWN, VALID, INVALID_EMPTY, INVALID_LEAK, INVALID_CONFED) CF_ENUM_PX(T_ENUM_AF, AF_, AFI_, IPV4, IPV6) CF_ENUM(T_ENUM_MPLS_POLICY, MPLS_POLICY_, NONE, STATIC, PREFIX, AGGREGATE, VRF) diff --git a/nest/route.h b/nest/route.h index a17ae696..c74c410f 100644 --- a/nest/route.h +++ b/nest/route.h @@ -321,7 +321,7 @@ static inline net *net_get(rtable *tab, const net_addr *addr) { return (net *) f net *net_get(rtable *tab, const net_addr *addr); net *net_route(rtable *tab, const net_addr *n); int net_roa_check(rtable *tab, const net_addr *n, u32 asn); -int aspa_check(rtable *tab, const struct adata *path); +enum aspa_result aspa_check(rtable *tab, const struct adata *path, bool force_upstream); rte *rte_find(net *net, struct rte_src *src); rte *rte_get_temp(struct rta *, struct rte_src *src); void rte_update2(struct channel *c, const net_addr *n, rte *new, struct rte_src *src); @@ -783,9 +783,12 @@ int rt_flowspec_check(rtable *tab_ip, rtable *tab_flow, const net_addr *n, rta * #define ROA_VALID 1 #define ROA_INVALID 2 -#define ASPA_UNKNOWN 0 -#define ASPA_VALID 1 -#define ASPA_INVALID 2 -#define ASPA_CONTAINS_CONFED 3 +enum aspa_result { + ASPA_UNKNOWN = 0, + ASPA_VALID, + ASPA_INVALID_EMPTY, + ASPA_INVALID_CONFED, + ASPA_INVALID_LEAK, +}; #endif diff --git a/nest/rt-table.c b/nest/rt-table.c index 2b930914..8cee48d6 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -353,44 +353,40 @@ net_roa_check(rtable *tab, const net_addr *n, u32 asn) * * Implements draft-ietf-sidrops-aspa-verification-16. */ -int aspa_check(rtable *tab, const adata *path) +enum aspa_result aspa_check(rtable *tab, const adata *path, bool force_upstream) { struct lp_state lps; lp_save(tmp_linpool, &lps); /* No support for confed paths */ if (as_path_contains_confed(path)) - return ASPA_CONTAINS_CONFED; + return ASPA_INVALID_CONFED; + + /* Check path length */ + uint len = as_path_getlen(path); + if (len == 0) + return ASPA_INVALID_EMPTY; /* Normalize the AS Path: drop stuffings */ - uint len = as_path_getlen(path); u32 *asns = alloca(sizeof(u32) * len); uint ppos = 0; - int nsz = 0; + uint nsz = 0; while (as_path_walk(path, &ppos, &asns[nsz])) if ((nsz == 0) || (asns[nsz] != asns[nsz-1])) nsz++; /* Find the provider blocks for every AS on the path * and check allowed directions */ - bool *up = alloca(sizeof(bool) * nsz); - bool *down = alloca(sizeof(bool) * nsz); - bool unknown_flag = false; + uint max_up = 0, min_up = 0, max_down = 0, min_down = 0; - for (int ap=0; aproutes) - { - /* No ASPA for this ASN, therefore UNKNOWN */ - unknown_flag = up[ap] = down[ap] = true; - continue; - } - up[ap] = down[ap] = false; + bool found = false, down = false, up = false; - for (rte *e = n->routes; e; e = e->next) + for (rte *e = (n ? n->routes: NULL); e; e = e->next) { if (!rte_is_valid(e)) continue; @@ -399,40 +395,80 @@ int aspa_check(rtable *tab, const adata *path) if (!ea) continue; + /* Actually found some ASPA */ + found = true; + for (uint i=0; i * sizeof(u32) < ea->u.ptr->length; i++) { if ((ap > 0) && ((u32 *) ea->u.ptr->data)[i] == asns[ap-1]) - down[ap] = true; + up = true; if ((ap + 1 < nsz) && ((u32 *) ea->u.ptr->data)[i] == asns[ap+1]) - up[ap] = true; + down = true; - if (down[ap] || up[ap]) - goto peering_found; + if (down && up) + /* Both peers found */ + goto end_of_aspa; } } -peering_found:; +end_of_aspa:; + + /* Fast path for the upstream check */ + if (force_upstream) + { + if (!found) + /* Move min-upstream */ + min_up = ap; + else if (ap && !up) + /* Exists but doesn't allow this upstream */ + return ASPA_INVALID_LEAK; + } + + /* Fast path for no ASPA here */ + else if (!found) + { + /* Extend max-downstream (min-downstream is stopped by unknown) */ + max_down = ap+1; + + /* Move min-upstream (can't include unknown) */ + min_up = ap; + } + + /* ASPA exists and downstream may be extended */ + else if (down) + { + /* Extending max-downstream always */ + max_down = ap+1; + + /* Extending min-downstream unless unknown seen */ + if (min_down == ap) + min_down = ap+1; + + /* Downstream only */ + if (!up) + min_up = max_up = ap; + } + + /* No extension for downstream, force upstream only from now */ + else + { + force_upstream = 1; + + /* Not even upstream, move the ending here */ + if (!up) + min_up = max_up = ap; + } } - /* Check whether the topology is first ramp up and then ramp down. */ - int up_end = 0; - while (up_end < nsz && up[up_end]) - up_end++; + /* Is the path surely valid? */ + if (min_up <= min_down) + return ASPA_VALID; - int down_end = nsz - 1; - while (down_end > 0 && down[down_end]) - down_end--; - - /* A significant overlap of obvious unknowns or misconfigured ASPAs. */ - if (up_end - down_end >= 2) + /* Is the path maybe valid? */ + if (max_up <= max_down) return ASPA_UNKNOWN; - /* The path has either a single transit provider, or a peering pair on top */ - else if (up_end - down_end >= 0) - return unknown_flag ? ASPA_UNKNOWN : ASPA_VALID; - - /* There is a gap between valid ramp up and valid ramp down */ - else - return ASPA_INVALID; + /* Now there is surely a valley there. */ + return ASPA_INVALID_LEAK; } /** diff --git a/proto/bgp/config.Y b/proto/bgp/config.Y index 4ce06488..7ae6cde9 100644 --- a/proto/bgp/config.Y +++ b/proto/bgp/config.Y @@ -39,7 +39,7 @@ CF_KEYWORDS(BGP, LOCAL, NEIGHBOR, AS, HOLD, TIME, CONNECT, RETRY, KEEPALIVE, CF_KEYWORDS(CEASE, PREFIX, LIMIT, HIT, ADMINISTRATIVE, SHUTDOWN, RESET, PEER, CONFIGURATION, CHANGE, DECONFIGURED, CONNECTION, REJECTED, COLLISION, - OUT, OF, RESOURCES) + OUT, OF, RESOURCES, ASPA_CHECK_CUSTOMER) %type bgp_cease_mask bgp_cease_list bgp_cease_flag bgp_role_name @@ -393,6 +393,31 @@ custom_attr: ATTRIBUTE BGP expr type symbol ';' { CF_ENUM(T_ENUM_BGP_ORIGIN, ORIGIN_, IGP, EGP, INCOMPLETE) +/* ASPA shortcuts */ +term: ASPA_CHECK '(' rtable ')' { $$ = + f_new_inst(FI_ASPA_CHECK_EXPLICIT, + f_new_inst(FI_EA_GET, + f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_ROUTE, .val.rte = NULL, }), + f_new_dynamic_attr(EAF_TYPE_AS_PATH, T_PATH, + EA_CODE(PROTOCOL_BGP, BA_AS_PATH)) + ), + f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_BOOL, .val.i = 0, }), + $3 + ); +} + +term: ASPA_CHECK_CUSTOMER '(' rtable ')' { $$ = + f_new_inst(FI_ASPA_CHECK_EXPLICIT, + f_new_inst(FI_EA_GET, + f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_ROUTE, .val.rte = NULL, }), + f_new_dynamic_attr(EAF_TYPE_AS_PATH, T_PATH, + EA_CODE(PROTOCOL_BGP, BA_AS_PATH)) + ), + f_new_inst(FI_CONSTANT, (struct f_val) { .type = T_BOOL, .val.i = 1, }), + $3 + ); +} + CF_CODE CF_END