From 60e9def9ef7b5d16f868b0fb4ab1192d59fd7541 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Sun, 9 Jan 2022 02:40:58 +0100 Subject: BGP: Add option 'free bind' The BGP 'free bind' option applies the IP_FREEBIND/IPV6_FREEBIND socket option for the BGP listening socket. Thanks to Alexander Zubkov for the idea. --- proto/bgp/bgp.h | 1 + 1 file changed, 1 insertion(+) (limited to 'proto/bgp/bgp.h') diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h index cca4b448..5e025ccd 100644 --- a/proto/bgp/bgp.h +++ b/proto/bgp/bgp.h @@ -86,6 +86,7 @@ struct bgp_config { int peer_type; /* Internal or external BGP (BGP_PT_*, optional) */ int multihop; /* Number of hops if multihop */ int strict_bind; /* Bind listening socket to local address */ + int free_bind; /* Bind listening socket with SKF_FREEBIND */ int ttl_security; /* Enable TTL security [RFC 5082] */ int compare_path_lengths; /* Use path lengths when selecting best route */ int med_metric; /* Compare MULTI_EXIT_DISC even between routes from differen ASes */ -- cgit v1.2.3 From 9dbb7eb6ebda016cd14ce8fef403c2b3f7bdd504 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Mon, 24 Jan 2022 03:44:21 +0100 Subject: BGP: Log route updates that were changed to withdraws Typical BGP error handling is treat-as-withdraw, where an invalid route is replaced with a withdraw. Log route network when it happens. --- proto/bgp/attrs.c | 4 ++++ proto/bgp/bgp.h | 1 + proto/bgp/packets.c | 8 +++++++- 3 files changed, 12 insertions(+), 1 deletion(-) (limited to 'proto/bgp/bgp.h') diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index 24ba00ba..0688c7cd 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -1845,6 +1845,10 @@ bgp_rt_notify(struct proto *P, struct channel *C, net *n, rte *new, rte *old) { struct ea_list *attrs = bgp_update_attrs(p, c, new, new->attrs->eattrs, bgp_linpool2); + /* Error during attribute processing */ + if (!attrs) + log(L_ERR "%s: Invalid route %N withdrawn", p->p.name, n->n.addr); + /* If attributes are invalid, we fail back to withdraw */ buck = attrs ? bgp_get_bucket(c, attrs) : bgp_get_withdraw_bucket(c); path = new->attrs->src->global_id; diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h index 5e025ccd..db05b693 100644 --- a/proto/bgp/bgp.h +++ b/proto/bgp/bgp.h @@ -423,6 +423,7 @@ struct bgp_parse_state { int as4_session; int add_path; int mpls; + int reach_nlri_step; u32 attrs_seen[256/32]; diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c index 99b5d5b4..9843a9e0 100644 --- a/proto/bgp/packets.c +++ b/proto/bgp/packets.c @@ -1335,7 +1335,7 @@ bgp_update_next_hop_none(struct bgp_export_state *s, eattr *a, ea_list **to) */ static void -bgp_rte_update(struct bgp_parse_state *s, net_addr *n, u32 path_id, rta *a0) +bgp_rte_update(struct bgp_parse_state *s, const net_addr *n, u32 path_id, rta *a0) { if (path_id != s->last_id) { @@ -1348,6 +1348,10 @@ bgp_rte_update(struct bgp_parse_state *s, net_addr *n, u32 path_id, rta *a0) if (!a0) { + /* Route update was changed to withdraw */ + if (s->err_withdraw && s->reach_nlri_step) + REPORT("Invalid route %N withdrawn", n); + /* Route withdraw */ rte_update3(&s->channel->c, n, NULL, s->last_src); return; @@ -2543,6 +2547,8 @@ bgp_rx_update(struct bgp_conn *conn, byte *pkt, uint len) if (s.mp_unreach_len) bgp_decode_nlri(&s, s.mp_unreach_af, s.mp_unreach_nlri, s.mp_unreach_len, NULL, NULL, 0); + s.reach_nlri_step = 1; + if (s.ip_reach_len) bgp_decode_nlri(&s, BGP_AF_IPV4, s.ip_reach_nlri, s.ip_reach_len, ea, s.ip_next_hop_data, s.ip_next_hop_len); -- cgit v1.2.3 From 963b2c7ce219df6bf9c179fff2dd2386cf26edf9 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Fri, 28 Jan 2022 05:35:22 +0100 Subject: BGP: Use proper class in attribute error messages Most error messages in attribute processing are in rx/decode step and these use L_REMOTE log class. But there are few that are in tx/export step and these should use L_ERR log class. Use tx-specific macro (REJECT()) in tx/export code and rename field err_withdraw to err_reject in struct bgp_export_state to ensure that appropriate error reporting macros are called in proper contexts. --- proto/bgp/attrs.c | 21 ++++++++++++--------- proto/bgp/bgp.h | 2 +- proto/bgp/packets.c | 13 ++++++++----- 3 files changed, 21 insertions(+), 15 deletions(-) (limited to 'proto/bgp/bgp.h') diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index 0688c7cd..968d7d2b 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -45,9 +45,9 @@ * * export - Hook that validates and normalizes attribute during export phase. * Receives eattr, may modify it (e.g., sort community lists for canonical - * representation), UNSET() it (e.g., skip empty lists), or WITHDRAW() it if - * necessary. May assume that eattr has value valid w.r.t. its type, but may be - * invalid w.r.t. BGP constraints. Optional. + * representation), UNSET() it (e.g., skip empty lists), or REJECT() the route + * if necessary. May assume that eattr has value valid w.r.t. its type, but may + * be invalid w.r.t. BGP constraints. Optional. * * encode - Hook that converts internal representation to external one during * packet writing. Receives eattr and puts it in the buffer (including attribute @@ -108,6 +108,9 @@ bgp_set_attr(ea_list **attrs, struct linpool *pool, uint code, uint flags, uintp #define UNSET(a) \ ({ a->type = EAF_TYPE_UNDEF; return; }) +#define REJECT(msg, args...) \ + ({ log(L_ERR "%s: " msg, s->proto->p.name, ## args); s->err_reject = 1; return; }) + #define NEW_BGP "Discarding %s attribute received from AS4-aware neighbor" #define BAD_EBGP "Discarding %s attribute received from EBGP neighbor" #define BAD_LENGTH "Malformed %s attribute - invalid length (%u)" @@ -380,7 +383,7 @@ static void bgp_export_origin(struct bgp_export_state *s, eattr *a) { if (a->u.data > 2) - WITHDRAW(BAD_VALUE, "ORIGIN", a->u.data); + REJECT(BAD_VALUE, "ORIGIN", a->u.data); } static void @@ -902,20 +905,20 @@ bgp_export_mpls_label_stack(struct bgp_export_state *s, eattr *a) /* Perhaps we should just ignore it? */ if (!s->mpls) - WITHDRAW("Unexpected MPLS stack"); + REJECT("Unexpected MPLS stack"); /* Empty MPLS stack is not allowed */ if (!lnum) - WITHDRAW("Malformed MPLS stack - empty"); + REJECT("Malformed MPLS stack - empty"); /* This is ugly, but we must ensure that labels fit into NLRI field */ if ((24*lnum + (net_is_vpn(n) ? 64 : 0) + net_pxlen(n)) > 255) - WITHDRAW("Malformed MPLS stack - too many labels (%u)", lnum); + REJECT("Malformed MPLS stack - too many labels (%u)", lnum); for (uint i = 0; i < lnum; i++) { if (labels[i] > 0xfffff) - WITHDRAW("Malformed MPLS stack - invalid label (%u)", labels[i]); + REJECT("Malformed MPLS stack - invalid label (%u)", labels[i]); /* TODO: Check for special-purpose label values? */ } @@ -1196,7 +1199,7 @@ bgp_export_attrs(struct bgp_export_state *s, ea_list *attrs) for (i = 0; i < count; i++) bgp_export_attr(s, &new->attrs[i], new); - if (s->err_withdraw) + if (s->err_reject) return NULL; return new; diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h index db05b693..08e751e7 100644 --- a/proto/bgp/bgp.h +++ b/proto/bgp/bgp.h @@ -397,7 +397,7 @@ struct bgp_export_state { int mpls; u32 attrs_seen[1]; - uint err_withdraw; + uint err_reject; uint local_next_hop; }; diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c index da5a6523..9d21fd34 100644 --- a/proto/bgp/packets.c +++ b/proto/bgp/packets.c @@ -932,6 +932,9 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, uint len) #define WITHDRAW(msg, args...) \ ({ REPORT(msg, ## args); s->err_withdraw = 1; return; }) +#define REJECT(msg, args...) \ + ({ log(L_ERR "%s: " msg, s->proto->p.name, ## args); s->err_reject = 1; return; }) + #define BAD_AFI "Unexpected AF <%u/%u> in UPDATE" #define BAD_NEXT_HOP "Invalid NEXT_HOP attribute" #define NO_NEXT_HOP "Missing NEXT_HOP attribute" @@ -1126,7 +1129,7 @@ bgp_update_next_hop_ip(struct bgp_export_state *s, eattr *a, ea_list **to) /* Check if next hop is valid */ a = bgp_find_attr(*to, BA_NEXT_HOP); if (!a) - WITHDRAW(NO_NEXT_HOP); + REJECT(NO_NEXT_HOP); ip_addr *nh = (void *) a->u.ptr->data; ip_addr peer = s->proto->remote_ip; @@ -1134,20 +1137,20 @@ bgp_update_next_hop_ip(struct bgp_export_state *s, eattr *a, ea_list **to) /* Forbid zero next hop */ if (ipa_zero2(nh[0]) && ((len != 32) || ipa_zero(nh[1]))) - WITHDRAW(BAD_NEXT_HOP " - zero address"); + REJECT(BAD_NEXT_HOP " - zero address"); /* Forbid next hop equal to neighbor IP */ if (ipa_equal(peer, nh[0]) || ((len == 32) && ipa_equal(peer, nh[1]))) - WITHDRAW(BAD_NEXT_HOP " - neighbor address %I", peer); + REJECT(BAD_NEXT_HOP " - neighbor address %I", peer); /* Forbid next hop with non-matching AF */ if ((ipa_is_ip4(nh[0]) != bgp_channel_is_ipv4(s->channel)) && !s->channel->ext_next_hop) - WITHDRAW(BAD_NEXT_HOP MISMATCHED_AF, nh[0], s->channel->desc->name); + REJECT(BAD_NEXT_HOP MISMATCHED_AF, nh[0], s->channel->desc->name); /* Just check if MPLS stack */ if (s->mpls && !bgp_find_attr(*to, BA_MPLS_LABEL_STACK)) - WITHDRAW(NO_LABEL_STACK); + REJECT(NO_LABEL_STACK); } static uint -- cgit v1.2.3 From 1f2eb2aca8e348fefc1822ec2adcad0cc97768d8 Mon Sep 17 00:00:00 2001 From: "Ondrej Zajicek (work)" Date: Mon, 20 Dec 2021 20:25:35 +0100 Subject: BGP: Implement flowspec validation procedure Implement flowspec validation procedure as described in RFC 8955 sec. 6 and RFC 9117. The Validation procedure enforces that only routers in the forwarding path for a network can originate flowspec rules for that network. The patch adds new mechanism for tracking inter-table dependencies, which is necessary as the flowspec validation depends on IP routes, and flowspec rules must be revalidated when best IP routes change. The validation procedure is disabled by default and requires that relevant IP table uses trie, as it uses interval queries for subnets. --- doc/bird.sgml | 28 +++- nest/route.h | 15 +++ nest/rt-table.c | 358 +++++++++++++++++++++++++++++++++++++++++++++++++--- proto/bgp/attrs.c | 4 + proto/bgp/bgp.c | 54 +++++++- proto/bgp/bgp.h | 6 +- proto/bgp/config.Y | 17 ++- proto/bgp/packets.c | 28 ++++ proto/pipe/pipe.c | 3 + 9 files changed, 487 insertions(+), 26 deletions(-) (limited to 'proto/bgp/bgp.h') diff --git a/doc/bird.sgml b/doc/bird.sgml index 39dadaf2..d1d2bdae 100644 --- a/doc/bird.sgml +++ b/doc/bird.sgml @@ -2274,6 +2274,7 @@ avoid routing loops. - BGP Large Communities Attribute - BGP Administrative Shutdown Communication - Default EBGP Route Propagation Behavior without Policies + - Revised Validation Procedure for BGP Flow Specifications Route selection rules @@ -2659,7 +2660,7 @@ using the following configuration parameters: