From ca09ec904113473f32ae29ba4ce68abe66548bf8 Mon Sep 17 00:00:00 2001 From: Michal Ruprich Date: Tue, 24 Aug 2021 14:10:06 +0200 Subject: [PATCH] Related: #1990858 - Fixing prefix-list duplication check --- 0006-prefix-list-duplication.patch | 343 +++++++++++++++++++++++++++++ frr.spec | 8 +- 2 files changed, 350 insertions(+), 1 deletion(-) create mode 100644 0006-prefix-list-duplication.patch diff --git a/0006-prefix-list-duplication.patch b/0006-prefix-list-duplication.patch new file mode 100644 index 0000000..c35aebc --- /dev/null +++ b/0006-prefix-list-duplication.patch @@ -0,0 +1,343 @@ +From 667dcc277c15c0bddc785f9b949d658f8d815818 Mon Sep 17 00:00:00 2001 +From: Igor Ryzhov +Date: Tue, 10 Aug 2021 21:46:37 +0300 +Subject: [PATCH] lib: fix prefix-list duplication check + +Currently, when we check the new prefix-list entry for duplication, we +only take filled in fields into account and ignore optional fields. +For example, if we already have `ip prefix-list A 0.0.0.0/0 le 32` and +we try to add `ip prefix-list A 0.0.0.0/0`, it is treated as duplicate. +We should always compare all prefix-list fields when doing the check. + +Fixes #9355. + +Signed-off-by: Igor Ryzhov +--- + lib/filter.h | 9 ++--- + lib/filter_cli.c | 102 ++++++++++------------------------------------- + lib/filter_nb.c | 85 ++++++++++++++++++++++----------------- + 3 files changed, 74 insertions(+), 122 deletions(-) + +diff --git a/lib/filter.h b/lib/filter.h +index 941fabd38b8..d1956ec019f 100644 +--- a/lib/filter.h ++++ b/lib/filter.h +@@ -207,11 +207,10 @@ struct plist_dup_args { + /** Entry action. */ + const char *pda_action; + +-#define PDA_MAX_VALUES 4 +- /** Entry XPath for value. */ +- const char *pda_xpath[PDA_MAX_VALUES]; +- /** Entry value to match. */ +- const char *pda_value[PDA_MAX_VALUES]; ++ bool any; ++ struct prefix prefix; ++ int ge; ++ int le; + + /** Duplicated entry found in list? */ + bool pda_found; +diff --git a/lib/filter_cli.c b/lib/filter_cli.c +index f030ce1b335..45c7544a3b4 100644 +--- a/lib/filter_cli.c ++++ b/lib/filter_cli.c +@@ -1196,11 +1196,9 @@ static int plist_remove_if_empty(struct vty *vty, const char *iptype, + + static int plist_remove(struct vty *vty, const char *iptype, const char *name, + const char *seq, const char *action, +- const char *prefix_str, const char *ge_str, +- const char *le_str) ++ union prefixconstptr prefix, int ge, int le) + { + int64_t sseq; +- int arg_idx = 0; + struct plist_dup_args pda = {}; + char xpath[XPATH_MAXLEN]; + char xpath_entry[XPATH_MAXLEN + 32]; +@@ -1225,43 +1223,13 @@ static int plist_remove(struct vty *vty, const char *iptype, const char *name, + pda.pda_type = iptype; + pda.pda_name = name; + pda.pda_action = action; +- if (prefix_str) { +- if (strmatch(iptype, "ipv4")) { +- pda.pda_xpath[arg_idx] = "./ipv4-prefix"; +- pda.pda_value[arg_idx] = prefix_str; +- arg_idx++; +- if (ge_str) { +- pda.pda_xpath[arg_idx] = +- "./ipv4-prefix-length-greater-or-equal"; +- pda.pda_value[arg_idx] = ge_str; +- arg_idx++; +- } +- if (le_str) { +- pda.pda_xpath[arg_idx] = +- "./ipv4-prefix-length-lesser-or-equal"; +- pda.pda_value[arg_idx] = le_str; +- arg_idx++; +- } +- } else { +- pda.pda_xpath[arg_idx] = "./ipv6-prefix"; +- pda.pda_value[arg_idx] = prefix_str; +- arg_idx++; +- if (ge_str) { +- pda.pda_xpath[arg_idx] = +- "./ipv6-prefix-length-greater-or-equal"; +- pda.pda_value[arg_idx] = ge_str; +- arg_idx++; +- } +- if (le_str) { +- pda.pda_xpath[arg_idx] = +- "./ipv6-prefix-length-lesser-or-equal"; +- pda.pda_value[arg_idx] = le_str; +- arg_idx++; +- } +- } ++ if (prefix.p) { ++ prefix_copy(&pda.prefix, prefix); ++ apply_mask(&pda.prefix); ++ pda.ge = ge; ++ pda.le = le; + } else { +- pda.pda_xpath[0] = "./any"; +- pda.pda_value[0] = ""; ++ pda.any = true; + } + + if (plist_is_dup(vty->candidate_config->dnode, &pda)) +@@ -1298,7 +1266,6 @@ DEFPY_YANG( + "Maximum prefix length\n") + { + int64_t sseq; +- int arg_idx = 0; + struct plist_dup_args pda = {}; + char xpath[XPATH_MAXLEN]; + char xpath_entry[XPATH_MAXLEN + 128]; +@@ -1312,24 +1279,11 @@ DEFPY_YANG( + pda.pda_name = name; + pda.pda_action = action; + if (prefix_str) { +- pda.pda_xpath[arg_idx] = "./ipv4-prefix"; +- pda.pda_value[arg_idx] = prefix_str; +- arg_idx++; +- if (ge_str) { +- pda.pda_xpath[arg_idx] = +- "./ipv4-prefix-length-greater-or-equal"; +- pda.pda_value[arg_idx] = ge_str; +- arg_idx++; +- } +- if (le_str) { +- pda.pda_xpath[arg_idx] = +- "./ipv4-prefix-length-lesser-or-equal"; +- pda.pda_value[arg_idx] = le_str; +- arg_idx++; +- } ++ prefix_copy(&pda.prefix, prefix); ++ pda.ge = ge; ++ pda.le = le; + } else { +- pda.pda_xpath[0] = "./any"; +- pda.pda_value[0] = ""; ++ pda.any = true; + } + + /* Duplicated entry without sequence, just quit. */ +@@ -1408,8 +1362,8 @@ DEFPY_YANG( + "Maximum prefix length to be matched\n" + "Maximum prefix length\n") + { +- return plist_remove(vty, "ipv4", name, seq_str, action, prefix_str, +- ge_str, le_str); ++ return plist_remove(vty, "ipv4", name, seq_str, action, ++ prefix_str ? prefix : NULL, ge, le); + } + + DEFPY_YANG( +@@ -1421,7 +1375,7 @@ DEFPY_YANG( + PREFIX_LIST_NAME_STR + ACCESS_LIST_SEQ_STR) + { +- return plist_remove(vty, "ipv4", name, seq_str, NULL, NULL, NULL, NULL); ++ return plist_remove(vty, "ipv4", name, seq_str, NULL, NULL, 0, 0); + } + + DEFPY_YANG( +@@ -1516,7 +1470,6 @@ DEFPY_YANG( + "Minimum prefix length\n") + { + int64_t sseq; +- int arg_idx = 0; + struct plist_dup_args pda = {}; + char xpath[XPATH_MAXLEN]; + char xpath_entry[XPATH_MAXLEN + 128]; +@@ -1530,24 +1483,11 @@ DEFPY_YANG( + pda.pda_name = name; + pda.pda_action = action; + if (prefix_str) { +- pda.pda_xpath[arg_idx] = "./ipv6-prefix"; +- pda.pda_value[arg_idx] = prefix_str; +- arg_idx++; +- if (ge_str) { +- pda.pda_xpath[arg_idx] = +- "./ipv6-prefix-length-greater-or-equal"; +- pda.pda_value[arg_idx] = ge_str; +- arg_idx++; +- } +- if (le_str) { +- pda.pda_xpath[arg_idx] = +- "./ipv6-prefix-length-lesser-or-equal"; +- pda.pda_value[arg_idx] = le_str; +- arg_idx++; +- } ++ prefix_copy(&pda.prefix, prefix); ++ pda.ge = ge; ++ pda.le = le; + } else { +- pda.pda_xpath[0] = "./any"; +- pda.pda_value[0] = ""; ++ pda.any = true; + } + + /* Duplicated entry without sequence, just quit. */ +@@ -1626,8 +1566,8 @@ DEFPY_YANG( + "Minimum prefix length to be matched\n" + "Minimum prefix length\n") + { +- return plist_remove(vty, "ipv6", name, seq_str, action, prefix_str, +- ge_str, le_str); ++ return plist_remove(vty, "ipv6", name, seq_str, action, ++ prefix_str ? prefix : NULL, ge, le); + } + + DEFPY_YANG( +@@ -1639,7 +1579,7 @@ DEFPY_YANG( + PREFIX_LIST_NAME_STR + ACCESS_LIST_SEQ_STR) + { +- return plist_remove(vty, "ipv6", name, seq_str, NULL, NULL, NULL, NULL); ++ return plist_remove(vty, "ipv6", name, seq_str, NULL, NULL, 0, 0); + } + + DEFPY_YANG( +diff --git a/lib/filter_nb.c b/lib/filter_nb.c +index 85805ffa47c..80ea7a57cb2 100644 +--- a/lib/filter_nb.c ++++ b/lib/filter_nb.c +@@ -387,10 +387,50 @@ static bool acl_zebra_is_dup(const struct lyd_node *dnode, + return acl_is_dup(entry_dnode, &ada); + } + ++static void plist_dnode_to_prefix(const struct lyd_node *dnode, bool *any, ++ struct prefix *p, int *ge, int *le) ++{ ++ *any = false; ++ *ge = 0; ++ *le = 0; ++ ++ if (yang_dnode_exists(dnode, "./any")) { ++ *any = true; ++ return; ++ } ++ ++ switch (yang_dnode_get_enum(dnode, "../type")) { ++ case YPLT_IPV4: ++ yang_dnode_get_prefix(p, dnode, "./ipv4-prefix"); ++ if (yang_dnode_exists(dnode, ++ "./ipv4-prefix-length-greater-or-equal")) ++ *ge = yang_dnode_get_uint8( ++ dnode, "./ipv4-prefix-length-greater-or-equal"); ++ if (yang_dnode_exists(dnode, ++ "./ipv4-prefix-length-lesser-or-equal")) ++ *le = yang_dnode_get_uint8( ++ dnode, "./ipv4-prefix-length-lesser-or-equal"); ++ break; ++ case YPLT_IPV6: ++ yang_dnode_get_prefix(p, dnode, "./ipv6-prefix"); ++ if (yang_dnode_exists(dnode, ++ "./ipv6-prefix-length-greater-or-equal")) ++ *ge = yang_dnode_get_uint8( ++ dnode, "./ipv6-prefix-length-greater-or-equal"); ++ if (yang_dnode_exists(dnode, ++ "./ipv6-prefix-length-lesser-or-equal")) ++ *le = yang_dnode_get_uint8( ++ dnode, "./ipv6-prefix-length-lesser-or-equal"); ++ break; ++ } ++} ++ + static int _plist_is_dup(const struct lyd_node *dnode, void *arg) + { + struct plist_dup_args *pda = arg; +- int idx; ++ struct prefix p; ++ int ge, le; ++ bool any; + + /* This entry is the caller, so skip it. */ + if (pda->pda_entry_dnode +@@ -400,19 +440,14 @@ static int _plist_is_dup(const struct lyd_node *dnode, void *arg) + if (strcmp(yang_dnode_get_string(dnode, "action"), pda->pda_action)) + return YANG_ITER_CONTINUE; + +- /* Check if all values match. */ +- for (idx = 0; idx < PDA_MAX_VALUES; idx++) { +- /* No more values. */ +- if (pda->pda_xpath[idx] == NULL) +- break; ++ plist_dnode_to_prefix(dnode, &any, &p, &ge, &le); + +- /* Not same type, just skip it. */ +- if (!yang_dnode_exists(dnode, pda->pda_xpath[idx])) ++ if (pda->any) { ++ if (!any) + return YANG_ITER_CONTINUE; +- +- /* Check if different value. */ +- if (strcmp(yang_dnode_get_string(dnode, pda->pda_xpath[idx]), +- pda->pda_value[idx])) ++ } else { ++ if (!prefix_same(&pda->prefix, &p) || pda->ge != ge ++ || pda->le != le) + return YANG_ITER_CONTINUE; + } + +@@ -439,17 +474,6 @@ static bool plist_is_dup_nb(const struct lyd_node *dnode) + const struct lyd_node *entry_dnode = + yang_dnode_get_parent(dnode, "entry"); + struct plist_dup_args pda = {}; +- int idx = 0, arg_idx = 0; +- static const char *entries[] = { +- "./ipv4-prefix", +- "./ipv4-prefix-length-greater-or-equal", +- "./ipv4-prefix-length-lesser-or-equal", +- "./ipv6-prefix", +- "./ipv6-prefix-length-greater-or-equal", +- "./ipv6-prefix-length-lesser-or-equal", +- "./any", +- NULL +- }; + + /* Initialize. */ + pda.pda_type = yang_dnode_get_string(entry_dnode, "../type"); +@@ -457,19 +481,8 @@ static bool plist_is_dup_nb(const struct lyd_node *dnode) + pda.pda_action = yang_dnode_get_string(entry_dnode, "action"); + pda.pda_entry_dnode = entry_dnode; + +- /* Load all values/XPaths. */ +- while (entries[idx] != NULL) { +- if (!yang_dnode_exists(entry_dnode, entries[idx])) { +- idx++; +- continue; +- } +- +- pda.pda_xpath[arg_idx] = entries[idx]; +- pda.pda_value[arg_idx] = +- yang_dnode_get_string(entry_dnode, entries[idx]); +- arg_idx++; +- idx++; +- } ++ plist_dnode_to_prefix(entry_dnode, &pda.any, &pda.prefix, &pda.ge, ++ &pda.le); + + return plist_is_dup(entry_dnode, &pda); + } diff --git a/frr.spec b/frr.spec index de826bc..81c4c25 100644 --- a/frr.spec +++ b/frr.spec @@ -5,7 +5,7 @@ Name: frr Version: 8.0 -Release: 2%{?checkout}%{?dist} +Release: 3%{?checkout}%{?dist} Summary: Routing daemon License: GPLv2+ URL: http://www.frrouting.org @@ -56,6 +56,9 @@ Patch0002: 0002-enable-openssl.patch Patch0003: 0003-disable-eigrp-crypto.patch Patch0004: 0004-fips-mode.patch Patch0005: 0005-use-python3.patch +#Adding a patch from frr after rebase +#We would have hit this surely later, better apply the patch now +Patch0006: 0006-prefix-list-duplication.patch %description FRRouting is free software that manages TCP/IP based routing protocols. It takes @@ -207,6 +210,9 @@ make check PYTHON=%{__python3} %{_tmpfilesdir}/%{name}.conf %changelog +* Mon Aug 16 2021 Michal Ruprich - 8.0-3 +- Related: #1990858 - Fixing prefix-list duplication check + * Thu Aug 12 2021 Michal Ruprich - 8.0-2 - Related: #1990858 - Frr needs higher version of libyang