From 816238029d76be4e45c704470761318c53a52745 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Mon, 22 Jul 2024 09:17:13 +0200 Subject: [PATCH] Fixed: [NETCONF error reply from failed leafref rquire-instance does not comply to RFC 7950](https://github.com/clicon/clixon/issues/53 --- CHANGELOG.md | 7 ++- lib/clixon/clixon_netconf_lib.h | 1 + lib/src/clixon_netconf_lib.c | 102 +++++++++++++++++++++++--------- lib/src/clixon_validate.c | 14 ++--- test/test_autocli_grouping.sh | 2 +- test/test_cli_leafref.sh | 2 +- test/test_leafref.sh | 5 +- test/test_leafref_augment.sh | 2 +- test/test_leafref_state.sh | 4 +- test/test_leafref_union.sh | 4 +- 10 files changed, 96 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b07a4ee..fd910167 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,8 +24,9 @@ Expected: October 2024 Users may have to change how they access the system +* NETCONF error returns of failed leafref references, see https://github.com/clicon/clixon/issues/536 * List pagination of large lists - * For backward-compatibility, mark the list with extension cl:list_pagination_partial_state extension + * For backward-compatibility, mark the list with extension cl:list-pagination-partial-state extension * New default is to use regular state read mechanism, which could have poorer performance but more functionality ### C/CLI-API changes on existing features @@ -37,6 +38,10 @@ Developers may need to change their code * Replace `y1 = NULL; y1 = yn_each(y0, y1)` with `int inext = 0; yn_iter(y0, &inext)` * Add `keyw`argument to `yang_stats()` +### Corrected Busg + +* Fixed: [NETCONF error reply from failed leafref rquire-instance does not comply to RFC 7950](https://github.com/clicon/clixon/issues/536) + ## 7.1.0 3 July 2024 diff --git a/lib/clixon/clixon_netconf_lib.h b/lib/clixon/clixon_netconf_lib.h index d87d11b2..fdcceb50 100644 --- a/lib/clixon/clixon_netconf_lib.h +++ b/lib/clixon/clixon_netconf_lib.h @@ -162,6 +162,7 @@ int netconf_bad_attribute(cbuf *cb, char *type, char *info, char *message); int netconf_bad_attribute_xml(cxobj **xret, char *type, char *info, char *message); int netconf_unknown_attribute(cbuf *cb, char *type, char *info, char *message); int netconf_missing_element(cbuf *cb, char *type, char *element, char *message); +int netconf_missing_yang_xml(cxobj **xret, char *path, char *app_tag, char *info, char *message); int netconf_missing_element_xml(cxobj **xret, char *type, char *element, char *message); int netconf_bad_element(cbuf *cb, char *type, char *info, char *element); int netconf_bad_element_xml(cxobj **xret, char *type, char *info, char *element); diff --git a/lib/src/clixon_netconf_lib.c b/lib/src/clixon_netconf_lib.c index c2d91922..e089b4a1 100644 --- a/lib/src/clixon_netconf_lib.c +++ b/lib/src/clixon_netconf_lib.c @@ -1060,32 +1060,30 @@ netconf_data_missing_xml(cxobj **xret, return retval; } -/*! Create Netconf data-missing / missing-choice as defeind in RFC 7950 15.6 +/*! Create Netconf data-missing / missing-choice as defined in RFC 7950 15.5/15.6 * - * If a NETCONF operation would result in configuration data where no - * nodes exists in a mandatory choice, the following error MUST be - * returned: - * @param[out] xret Error XML tree. Free with xml_free after use - * @param[in] x Element with missing choice - * @param[in] name Name of missing mandatory choice - * @param[in] message Error message - * @retval 0 OK - * @retval -1 Error + * @param[out] xret Error XML tree. Free with xml_free after use + * @param[in] path Path + * @param[in] app-tag Error app-tag + * @param[in] info Error info + * @param[in] message Error message + * @retval 0 OK + * @retval -1 Error */ int -netconf_missing_choice_xml(cxobj **xret, - cxobj *x, - char *name, - char *message) +netconf_missing_yang_xml(cxobj **xret, + char *path, + char *app_tag, + char *info, + char *message) { int retval = -1; char *encstr = NULL; cxobj *xerr; - char *path = NULL; char *encpath = NULL; - if (xret == NULL || name == NULL){ - clixon_err(OE_NETCONF, EINVAL, "xret or name is NULL"); + if (xret == NULL || path == NULL){ + clixon_err(OE_NETCONF, EINVAL, "xret or path is NULL"); goto done; } if (*xret == NULL){ @@ -1098,21 +1096,23 @@ netconf_missing_choice_xml(cxobj **xret, goto done; if ((xerr = xml_new("rpc-error", *xret, CX_ELMNT)) == NULL) goto done; - /* error-path: Path to the element with the missing choice. */ - if (xml2xpath(x, NULL, 0, 0, &path) < 0) - goto done; if (xml_chardata_encode(&encpath, 0, "%s", path) < 0) goto done; if (clixon_xml_parse_va(YB_NONE, NULL, &xerr, NULL, "application" "data-missing" - "missing-choice" - "%s" - "%s" - "error", - encpath, - YANG_XML_NAMESPACE, - name) < 0) + "%s" + "%s", + app_tag, + encpath) < 0) + goto done; + if (info) { + if (clixon_xml_parse_va(YB_NONE, NULL, &xerr, NULL, + "%s", info) < 0) + goto done; + } + if (clixon_xml_parse_va(YB_NONE, NULL, &xerr, NULL, + "error") < 0) goto done; if (message){ if (xml_chardata_encode(&encstr, 0, "%s", message) < 0) @@ -1123,8 +1123,6 @@ netconf_missing_choice_xml(cxobj **xret, } retval = 0; done: - if (path) - free(path); if (encstr) free(encstr); if (encpath) @@ -1132,6 +1130,52 @@ netconf_missing_choice_xml(cxobj **xret, return retval; } +/*! Create Netconf data-missing / missing-choice as defined in RFC 7950 15.6 + * + * If a NETCONF operation would result in configuration data where no + * nodes exists in a mandatory choice, the following error MUST be + * returned: + * @param[out] xret Error XML tree. Free with xml_free after use + * @param[in] x Element with missing choice + * @param[in] name Name of missing mandatory choice + * @param[in] message Error message + * @retval 0 OK + * @retval -1 Error + */ + +int +netconf_missing_choice_xml(cxobj **xret, + cxobj *x, + char *name, + char *message) +{ + int retval = -1; + cbuf *cbinfo = NULL; + char *path = NULL; + + if (name == NULL){ + clixon_err(OE_NETCONF, EINVAL, "xret or name is NULL"); + goto done; + } + /* error-path: Path to the element. */ + if (xml2xpath(x, NULL, 0, 0, &path) < 0) + goto done; + if ((cbinfo = cbuf_new()) == NULL){ + clixon_err(OE_UNIX, errno, "cbuf_new"); + goto done; + } + cprintf(cbinfo, "%s", YANG_XML_NAMESPACE, name); + if (netconf_missing_yang_xml(xret, path, "missing-choice", cbuf_get(cbinfo), message) < 0) + goto done; + retval = 0; + done: + if (path) + free(path); + if (cbinfo) + cbuf_free(cbinfo); + return retval; +} + /*! Create Netconf operation-not-supported error XML according to RFC 6241 App A * * Request could not be completed because the requested operation is not diff --git a/lib/src/clixon_validate.c b/lib/src/clixon_validate.c index bc62b38d..e6192797 100644 --- a/lib/src/clixon_validate.c +++ b/lib/src/clixon_validate.c @@ -133,7 +133,6 @@ validate_leafref(cxobj *xt, cvec *nsc = NULL; cbuf *cberr = NULL; char *path_arg; - yang_stmt *ymod; cg_var *cv; int require_instance = 1; @@ -179,13 +178,12 @@ validate_leafref(cxobj *xt, clixon_err(OE_UNIX, errno, "cbuf_new"); goto done; } - ymod = ys_module(ys); - cprintf(cberr, "Leafref validation failed: No leaf %s matching path %s in %s.yang:%d", - leafrefbody, - path_arg, - yang_argument_get(ymod), - yang_linenum_get(ys)); - if (xret && netconf_bad_element_xml(xret, "application", leafrefbody, cbuf_get(cberr)) < 0) + /* RFC 7950 15.5 requires: + error-tag: data-missing + error-app-tag: instance-required + error-path: Path to the instance-identifier or leafref leaf. + */ + if (xret && netconf_missing_yang_xml(xret, path_arg, "instance-required", leafrefbody, NULL) < 0) goto done; goto fail; } diff --git a/test/test_autocli_grouping.sh b/test/test_autocli_grouping.sh index 7b834d08..f8646a5f 100755 --- a/test/test_autocli_grouping.sh +++ b/test/test_autocli_grouping.sh @@ -243,7 +243,7 @@ EOF expectpart "$($clixon_cli -f $cfg -1 set table parameter x index1 a scope 43)" 0 "" new "validate expect fail" - expectpart "$($clixon_cli -f $cfg -1 validate 2>&1)" 255 "bad-element Leafref validation failed: No leaf 43 matching path" + expectpart "$($clixon_cli -f $cfg -1 validate 2>&1)" 255 "data-missing" "instance-required : ../../value0" new "set leafref expect fail" expectpart "$($clixon_cli -f $cfg -1 set table parameter x value0 43)" 0 "" diff --git a/test/test_cli_leafref.sh b/test/test_cli_leafref.sh index 886e2d55..1e8a92d5 100755 --- a/test/test_cli_leafref.sh +++ b/test/test_cli_leafref.sh @@ -288,7 +288,7 @@ new "set leafref require-instance 99 (non-existent)" expectpart "$($clixon_cli -1 -f $cfg set leafrefsreqinst leafref 99)" 0 "^"$ new "cli validate expect failure" -expectpart "$($clixon_cli -1 -f $cfg -l o validate)" 255 "Leafref validation failed: No leaf 99 matching path" +expectpart "$($clixon_cli -1 -f $cfg -l o validate)" 255 "data-missing 99: instance-required : /table/parameter/name" new "cli discard" expectpart "$($clixon_cli -1 -f $cfg -l o discard)" 0 "" diff --git a/test/test_leafref.sh b/test/test_leafref.sh index 7c00fb55..512af586 100755 --- a/test/test_leafref.sh +++ b/test/test_leafref.sh @@ -145,7 +145,8 @@ new "leafref add non-existing ref" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "eth3
10.0.4.6
" "" "" new "leafref validate" -expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationbad-elementeth3errorLeafref validation failed: No leaf eth3 matching path /if:interfaces/if:interface/if:name in example.yang:[0-9]*" "" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationdata-missinginstance-required/if:interfaces/if:interface/if:nameeth3error" "" #new "leafref wrong ref" #expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "eth3
10.0.4.6
" "" "" @@ -175,7 +176,7 @@ new "leafref delete leaf" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "eth0" "" "" new "leafref validate (should fail)" -expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationbad-elementeth0errorLeafref validation failed: No leaf eth0 matching path /if:interfaces/if:interface/if:name in example.yang:[0-9]*" "" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationdata-missinginstance-required/if:interfaces/if:interface/if:nameeth0error" "" new "leafref discard-changes" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" diff --git a/test/test_leafref_augment.sh b/test/test_leafref_augment.sh index afe93c64..5866df82 100755 --- a/test/test_leafref_augment.sh +++ b/test/test_leafref_augment.sh @@ -211,7 +211,7 @@ new "leafref augment+leafref config wrong ref" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "$XML" "" "" new "leafref augment+leafref validate wrong ref" -expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationbad-elementxxxerrorLeafref validation failed: No leaf xxx matching path /ex:sender/ex:name in augment.yang:[0-9]*" "" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationdata-missinginstance-required/ex:sender/ex:namexxxerror" "" new "netconf discard-changes" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" diff --git a/test/test_leafref_state.sh b/test/test_leafref_state.sh index 842f0ad0..dfbbfcdf 100755 --- a/test/test_leafref_state.sh +++ b/test/test_leafref_state.sh @@ -202,10 +202,10 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" " # Leafref wrong internal: state references x but config contains only y new "netconf get / config+state should fail" -expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationoperation-failedxerrorLeafref validation failed: No leaf x matching path /ex:sender-config/ex:name in leafref.yang:[0-9]*. Internal error, state callback returned invalid XML" "" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationoperation-failedinstance-required/ex:sender-config/ex:namexerror" "" new "netconf get / state-only should fail" -expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationoperation-failedxerrorLeafref validation failed: No leaf x matching path /ex:sender-config/ex:name in leafref.yang:[0-9]*. Internal error, state callback returned invalid XML" "" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationoperation-failedinstance-required/ex:sender-config/ex:namexerror" "" new "netconf get / config-only ok" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "y" diff --git a/test/test_leafref_union.sh b/test/test_leafref_union.sh index f72ecd8f..1100d65e 100755 --- a/test/test_leafref_union.sh +++ b/test/test_leafref_union.sh @@ -150,7 +150,7 @@ new "netconf set union 66" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "66none" "" "" new "netconf validate not ok" -expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationbad-element66errorLeafref validation failed: No leaf 66 matching path ../y" "" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationdata-missinginstance-required../y66error" new "netconf discard-changes" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "]]>]]>" "" "" @@ -186,7 +186,7 @@ new "netconf set union 99" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "99none" "" "" new "netconf validate not ok" -expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationbad-element99errorLeafref validation failed" "" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "applicationdata-missinginstance-required../x99error" "" if [ $BE -ne 0 ]; then new "Kill backend"