From 7fcb24bbaa91762965ad15aff10956919fdc3c08 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Wed, 2 Jan 2019 18:37:00 -0200 Subject: [PATCH] zebra: reject routes without nexthops Routes without nexthops don't make any sense, so we need to reject them otherwise weird things can happen. NOTE: blackhole routes aren't nexthop-less, they do have a single nexthop of type NEXTHOP_TYPE_BLACKHOLE. Signed-off-by: Renato Westphal --- zebra/zapi_msg.c | 265 ++++++++++++++++++++++--------------------- zebra/zebra_errors.c | 9 ++ zebra/zebra_errors.h | 1 + 3 files changed, 144 insertions(+), 131 deletions(-) diff --git a/zebra/zapi_msg.c b/zebra/zapi_msg.c index 8fe7031abe..f049a8d1a2 100644 --- a/zebra/zapi_msg.c +++ b/zebra/zapi_msg.c @@ -1395,147 +1395,150 @@ static void zread_route_add(ZAPI_HANDLER_ARGS) else re->table = zvrf->table_id; + if (!CHECK_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP) + || api.nexthop_num == 0) { + char buf_prefix[PREFIX_STRLEN]; + + prefix2str(&api.prefix, buf_prefix, sizeof(buf_prefix)); + flog_warn(EC_ZEBRA_RX_ROUTE_NO_NEXTHOPS, + "%s: received a route without nexthops for prefix %s", + __func__, buf_prefix); + XFREE(MTYPE_RE, re); + return; + } + /* * TBD should _all_ of the nexthop add operations use * api_nh->vrf_id instead of re->vrf_id ? I only changed * for cases NEXTHOP_TYPE_IPV4 and NEXTHOP_TYPE_IPV6. */ - if (CHECK_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP)) - for (i = 0; i < api.nexthop_num; i++) { - api_nh = &api.nexthops[i]; - ifindex_t ifindex = 0; - - if (IS_ZEBRA_DEBUG_RECV) - zlog_debug("nh type %d", api_nh->type); - - switch (api_nh->type) { - case NEXTHOP_TYPE_IFINDEX: - nexthop = route_entry_nexthop_ifindex_add( - re, api_nh->ifindex, api_nh->vrf_id); - break; - case NEXTHOP_TYPE_IPV4: - if (IS_ZEBRA_DEBUG_RECV) { - char nhbuf[INET6_ADDRSTRLEN] = {0}; - - inet_ntop(AF_INET, &api_nh->gate.ipv4, - nhbuf, INET6_ADDRSTRLEN); - zlog_debug("%s: nh=%s, vrf_id=%d", - __func__, nhbuf, - api_nh->vrf_id); - } - nexthop = route_entry_nexthop_ipv4_add( - re, &api_nh->gate.ipv4, NULL, - api_nh->vrf_id); - break; - case NEXTHOP_TYPE_IPV4_IFINDEX: - - memset(&vtep_ip, 0, sizeof(struct ipaddr)); - if (CHECK_FLAG(api.flags, - ZEBRA_FLAG_EVPN_ROUTE)) { - ifindex = get_l3vni_svi_ifindex(vrf_id); - } else { - ifindex = api_nh->ifindex; - } - - if (IS_ZEBRA_DEBUG_RECV) { - char nhbuf[INET6_ADDRSTRLEN] = {0}; - - inet_ntop(AF_INET, &api_nh->gate.ipv4, - nhbuf, INET6_ADDRSTRLEN); - zlog_debug( - "%s: nh=%s, vrf_id=%d (re->vrf_id=%d), ifindex=%d", - __func__, nhbuf, api_nh->vrf_id, - re->vrf_id, ifindex); - } - nexthop = route_entry_nexthop_ipv4_ifindex_add( - re, &api_nh->gate.ipv4, NULL, ifindex, - api_nh->vrf_id); - - /* if this an EVPN route entry, - * program the nh as neigh - */ - if (CHECK_FLAG(api.flags, - ZEBRA_FLAG_EVPN_ROUTE)) { - SET_FLAG(nexthop->flags, - NEXTHOP_FLAG_EVPN_RVTEP); - vtep_ip.ipa_type = IPADDR_V4; - memcpy(&(vtep_ip.ipaddr_v4), - &(api_nh->gate.ipv4), - sizeof(struct in_addr)); - zebra_vxlan_evpn_vrf_route_add( - vrf_id, &api_nh->rmac, &vtep_ip, - &api.prefix); - } - break; - case NEXTHOP_TYPE_IPV6: - nexthop = route_entry_nexthop_ipv6_add( - re, &api_nh->gate.ipv6, api_nh->vrf_id); - break; - case NEXTHOP_TYPE_IPV6_IFINDEX: - memset(&vtep_ip, 0, sizeof(struct ipaddr)); - if (CHECK_FLAG(api.flags, - ZEBRA_FLAG_EVPN_ROUTE)) { - ifindex = get_l3vni_svi_ifindex(vrf_id); - } else { - ifindex = api_nh->ifindex; - } - - nexthop = route_entry_nexthop_ipv6_ifindex_add( - re, &api_nh->gate.ipv6, ifindex, - api_nh->vrf_id); - - /* if this an EVPN route entry, - * program the nh as neigh - */ - if (CHECK_FLAG(api.flags, - ZEBRA_FLAG_EVPN_ROUTE)) { - SET_FLAG(nexthop->flags, - NEXTHOP_FLAG_EVPN_RVTEP); - vtep_ip.ipa_type = IPADDR_V6; - memcpy(&vtep_ip.ipaddr_v6, - &(api_nh->gate.ipv6), - sizeof(struct in6_addr)); - zebra_vxlan_evpn_vrf_route_add( - vrf_id, &api_nh->rmac, &vtep_ip, - &api.prefix); - } - break; - case NEXTHOP_TYPE_BLACKHOLE: - nexthop = route_entry_nexthop_blackhole_add( - re, api_nh->bh_type); - break; + for (i = 0; i < api.nexthop_num; i++) { + api_nh = &api.nexthops[i]; + ifindex_t ifindex = 0; + + if (IS_ZEBRA_DEBUG_RECV) + zlog_debug("nh type %d", api_nh->type); + + switch (api_nh->type) { + case NEXTHOP_TYPE_IFINDEX: + nexthop = route_entry_nexthop_ifindex_add( + re, api_nh->ifindex, api_nh->vrf_id); + break; + case NEXTHOP_TYPE_IPV4: + if (IS_ZEBRA_DEBUG_RECV) { + char nhbuf[INET6_ADDRSTRLEN] = {0}; + + inet_ntop(AF_INET, &api_nh->gate.ipv4, nhbuf, + INET6_ADDRSTRLEN); + zlog_debug("%s: nh=%s, vrf_id=%d", __func__, + nhbuf, api_nh->vrf_id); } + nexthop = route_entry_nexthop_ipv4_add( + re, &api_nh->gate.ipv4, NULL, api_nh->vrf_id); + break; + case NEXTHOP_TYPE_IPV4_IFINDEX: - if (!nexthop) { - flog_warn( - EC_ZEBRA_NEXTHOP_CREATION_FAILED, - "%s: Nexthops Specified: %d but we failed to properly create one", - __PRETTY_FUNCTION__, api.nexthop_num); - nexthops_free(re->ng.nexthop); - XFREE(MTYPE_RE, re); - return; + memset(&vtep_ip, 0, sizeof(struct ipaddr)); + if (CHECK_FLAG(api.flags, ZEBRA_FLAG_EVPN_ROUTE)) { + ifindex = get_l3vni_svi_ifindex(vrf_id); + } else { + ifindex = api_nh->ifindex; } - /* MPLS labels for BGP-LU or Segment Routing */ - if (CHECK_FLAG(api.message, ZAPI_MESSAGE_LABEL) - && api_nh->type != NEXTHOP_TYPE_IFINDEX - && api_nh->type != NEXTHOP_TYPE_BLACKHOLE) { - enum lsp_types_t label_type; - - label_type = - lsp_type_from_re_type(client->proto); - - if (IS_ZEBRA_DEBUG_RECV) { - zlog_debug( - "%s: adding %d labels of type %d (1st=%u)", - __func__, api_nh->label_num, - label_type, api_nh->labels[0]); - } - - nexthop_add_labels(nexthop, label_type, - api_nh->label_num, - &api_nh->labels[0]); + + if (IS_ZEBRA_DEBUG_RECV) { + char nhbuf[INET6_ADDRSTRLEN] = {0}; + + inet_ntop(AF_INET, &api_nh->gate.ipv4, nhbuf, + INET6_ADDRSTRLEN); + zlog_debug( + "%s: nh=%s, vrf_id=%d (re->vrf_id=%d), ifindex=%d", + __func__, nhbuf, api_nh->vrf_id, + re->vrf_id, ifindex); + } + nexthop = route_entry_nexthop_ipv4_ifindex_add( + re, &api_nh->gate.ipv4, NULL, ifindex, + api_nh->vrf_id); + + /* if this an EVPN route entry, + * program the nh as neigh + */ + if (CHECK_FLAG(api.flags, ZEBRA_FLAG_EVPN_ROUTE)) { + SET_FLAG(nexthop->flags, + NEXTHOP_FLAG_EVPN_RVTEP); + vtep_ip.ipa_type = IPADDR_V4; + memcpy(&(vtep_ip.ipaddr_v4), + &(api_nh->gate.ipv4), + sizeof(struct in_addr)); + zebra_vxlan_evpn_vrf_route_add( + vrf_id, &api_nh->rmac, &vtep_ip, + &api.prefix); + } + break; + case NEXTHOP_TYPE_IPV6: + nexthop = route_entry_nexthop_ipv6_add( + re, &api_nh->gate.ipv6, api_nh->vrf_id); + break; + case NEXTHOP_TYPE_IPV6_IFINDEX: + memset(&vtep_ip, 0, sizeof(struct ipaddr)); + if (CHECK_FLAG(api.flags, ZEBRA_FLAG_EVPN_ROUTE)) { + ifindex = get_l3vni_svi_ifindex(vrf_id); + } else { + ifindex = api_nh->ifindex; + } + + nexthop = route_entry_nexthop_ipv6_ifindex_add( + re, &api_nh->gate.ipv6, ifindex, + api_nh->vrf_id); + + /* if this an EVPN route entry, + * program the nh as neigh + */ + if (CHECK_FLAG(api.flags, ZEBRA_FLAG_EVPN_ROUTE)) { + SET_FLAG(nexthop->flags, + NEXTHOP_FLAG_EVPN_RVTEP); + vtep_ip.ipa_type = IPADDR_V6; + memcpy(&vtep_ip.ipaddr_v6, &(api_nh->gate.ipv6), + sizeof(struct in6_addr)); + zebra_vxlan_evpn_vrf_route_add( + vrf_id, &api_nh->rmac, &vtep_ip, + &api.prefix); } + break; + case NEXTHOP_TYPE_BLACKHOLE: + nexthop = route_entry_nexthop_blackhole_add( + re, api_nh->bh_type); + break; + } + + if (!nexthop) { + flog_warn( + EC_ZEBRA_NEXTHOP_CREATION_FAILED, + "%s: Nexthops Specified: %d but we failed to properly create one", + __PRETTY_FUNCTION__, api.nexthop_num); + nexthops_free(re->ng.nexthop); + XFREE(MTYPE_RE, re); + return; } + /* MPLS labels for BGP-LU or Segment Routing */ + if (CHECK_FLAG(api.message, ZAPI_MESSAGE_LABEL) + && api_nh->type != NEXTHOP_TYPE_IFINDEX + && api_nh->type != NEXTHOP_TYPE_BLACKHOLE) { + enum lsp_types_t label_type; + + label_type = lsp_type_from_re_type(client->proto); + + if (IS_ZEBRA_DEBUG_RECV) { + zlog_debug( + "%s: adding %d labels of type %d (1st=%u)", + __func__, api_nh->label_num, label_type, + api_nh->labels[0]); + } + + nexthop_add_labels(nexthop, label_type, + api_nh->label_num, + &api_nh->labels[0]); + } + } if (CHECK_FLAG(api.message, ZAPI_MESSAGE_DISTANCE)) re->distance = api.distance; diff --git a/zebra/zebra_errors.c b/zebra/zebra_errors.c index d7c17829cd..cb5f30df1f 100644 --- a/zebra/zebra_errors.c +++ b/zebra/zebra_errors.c @@ -504,6 +504,15 @@ static struct log_ref ferr_zebra_err[] = { .suggestion = "Check configuration values for correctness. If they are correct, report this as a bug.", }, + { + .code = EC_ZEBRA_RX_ROUTE_NO_NEXTHOPS, + .title = + "Zebra received an installation request for a route without nexthops", + .description = + "Zebra received a message from a client requesting a route installation, but the route is invalid since it doesn't have any nexthop address or interface.", + .suggestion = + "This is a bug; please report it.", + }, { .code = EC_ZEBRA_RX_SRCDEST_WRONG_AFI, .title = diff --git a/zebra/zebra_errors.h b/zebra/zebra_errors.h index c3cdc4ed42..0af5f8a551 100644 --- a/zebra/zebra_errors.h +++ b/zebra/zebra_errors.h @@ -96,6 +96,7 @@ enum zebra_log_refs { EC_ZEBRA_VRF_NOT_FOUND, EC_ZEBRA_MORE_NH_THAN_MULTIPATH, EC_ZEBRA_NEXTHOP_CREATION_FAILED, + EC_ZEBRA_RX_ROUTE_NO_NEXTHOPS, EC_ZEBRA_RX_SRCDEST_WRONG_AFI, EC_ZEBRA_PSEUDOWIRE_EXISTS, EC_ZEBRA_PSEUDOWIRE_UNINSTALL_NOT_FOUND, -- 2.39.5