diff --git a/CHANGELOG.md b/CHANGELOG.md index a56d6d4d..dec3427d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,9 @@ Expected: Early March 2020 ### Minor changes +* Sanity check of mandatory key statement for Yang LISTs. + * If fails, exit with error message, eg: `Yang error: Sanity check failed: LIST vsDataContainer lacks key statement which MUST be present (See RFC 7950 Sec 7.8.2)` + * Can be disabled by setting `CLICON_CLICON_YANG_LIST_CHECK` to `false` * Replaced compile option `VALIDATE_STATE_XML` with runtime option `CLICON_VALIDATE_STATE_XML`. * Memory footprint * Do not autopopulate namespace cache, instead use on-demand, see `xml2ns()`. @@ -90,8 +93,9 @@ Expected: Early March 2020 ### Corrected Bugs -* Recursive (erroneous) Yang specs with recursive grouping/use statement is now fixed: instead of stack overflow, you get an error message and an exit -* Fixed: Search function checked only own not for config false statement, should have checked all ancestors. This may affect some state returned in GET calls +* Yang specs with recursive grouping/use statement is now fixed: instead of stack overflow, you get an error message and an exit +* Fixed: Some state data was sorted but should not have been. + * Search function checked only own not for config false statement, should have checked all ancestors. * Fixed: Some restconf errors were wrongly formatted such as: `{"ietf-restconf:errors":{"error":{"rpc-error":` . There should be no `"rpc-error"` level. * Fixed: Enabling modstate (CLICON_XMLDB_MODSTATE), changing a revision on a yang, and restarting made the backend daemon exit at start (thanks Matt) * Also: ensure to load `ietf-yang-library.yang ` if CLICON_XMLDB_MODSTATE is set diff --git a/README.md b/README.md index 54b606af..3001bf54 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ Clixon is a YANG-based configuration manager, with interactive CLI, NETCONF and RESTCONF interfaces, an embedded database and transaction mechanism. -See [main documentation](https://clixon-docs.readthedocs.io) and [project page](https://www.clicon.org). +See [main documentation](https://clixon-docs.readthedocs.io), [project page](https://www.clicon.org) and [examples](https://github.com/clicon/clixon-examples). Clixon is open-source and dual licensed. Either Apache License, Version 2.0 or GNU General Public License Version 2; you choose. diff --git a/doc/DEVELOP.md b/doc/DEVELOP.md index d615e6ae..fc309dc8 100644 --- a/doc/DEVELOP.md +++ b/doc/DEVELOP.md @@ -56,6 +56,10 @@ configure.ac --. Note: remember to run autoheader sometimes (when?) And when you do note (https://github.com/olofhagsand/cligen/issues/17) which states that cligen_custom.h should be in quote. +Get config.sub and config.guess: +$ wget -O config.guess 'https://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;f=config.guess;hb=HEAD' +$ wget -O config.sub 'https://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;f=config.sub;hb=HEAD' + ## Debug How to debug diff --git a/example/hello/Makefile.in b/example/hello/Makefile.in index 9bea26cb..de0acb4d 100644 --- a/example/hello/Makefile.in +++ b/example/hello/Makefile.in @@ -125,6 +125,7 @@ install: $(YANGSPECS) $(CLISPECS) $(PLUGINS) $(APPNAME).xml install -d -m 0755 $(DESTDIR)$(libdir)/$(APPNAME)/clispec install -m 0644 $(CLISPECS) $(DESTDIR)$(libdir)/$(APPNAME)/clispec install -d -m 0755 $(DESTDIR)$(datarootdir)/$(APPNAME)/yang + install -d -m 0755 $(DESTDIR)$(YANG_INSTALLDIR) install -m 0644 $(YANGSPECS) $(DESTDIR)$(YANG_INSTALLDIR) install -d -m 0755 $(DESTDIR)$(localstatedir)/$(APPNAME) diff --git a/example/main/Makefile.in b/example/main/Makefile.in index cc8cbbfb..21a03083 100644 --- a/example/main/Makefile.in +++ b/example/main/Makefile.in @@ -129,7 +129,8 @@ install: $(YANGSPECS) $(CLISPECS) $(PLUGINS) $(APPNAME).xml install -m 0644 $(APPNAME).xml $(DESTDIR)$(sysconfdir) # install -m 0644 $(APPNAME).xml $(DESTDIR)$(CLIXON_DEFAULT_CONFIG) install -d -m 0755 $(DESTDIR)$(datarootdir)/$(APPNAME)/yang - install -m 0644 $(YANGSPECS) $(DESTDIR)$(DESTDIR)$(YANG_INSTALLDIR) + install -d -m 0755 $(DESTDIR)$(YANG_INSTALLDIR) + install -m 0644 $(YANGSPECS) $(DESTDIR)$(YANG_INSTALLDIR) install -d -m 0755 $(DESTDIR)$(libdir)/$(APPNAME)/cli install -m 0644 $(INSTALLFLAGS) $(CLI_PLUGIN) $(DESTDIR)$(libdir)/$(APPNAME)/cli install -d -m 0755 $(DESTDIR)$(libdir)/$(APPNAME)/backend diff --git a/lib/clixon/clixon_yang.h b/lib/clixon/clixon_yang.h index 06349b7e..23a702d5 100644 --- a/lib/clixon/clixon_yang.h +++ b/lib/clixon/clixon_yang.h @@ -155,8 +155,32 @@ struct xml; typedef struct yang_stmt yang_stmt; /* Defined in clixon_yang_internal */ +/*! Yang apply function worker + * @param[in] yn yang node + * @param[in] arg Argument + * @retval -1 Error, abort + * @retval 0 OK, continue with next + * @retval n OK, abort traversal and return to caller with "n" + */ typedef int (yang_applyfn_t)(yang_stmt *ys, void *arg); + +/* Yang data definition statement + * See RFC 7950 Sec 3: + * o data definition statement: A statement that defines new data + * nodes. One of "container", "leaf", "leaf-list", "list", "choice", + * "case", "augment", "uses", "anydata", and "anyxml". + */ +#define yang_datadefinition(y) (yang_datanode(y) || yang_keyword_get(y) == Y_CHOICE || yang_keyword_get(y) == Y_CASE || yang_keyword_get(y) == Y_AUGMENT || yang_keyword_get(y) == Y_USES) + +/* Yang schema node . + * See RFC 7950 Sec 3: + * o schema node: A node in the schema tree. One of action, container, + * leaf, leaf-list, list, choice, case, rpc, input, output, + * notification, anydata, and anyxml. + */ +#define yang_schemanode(y) (yang_datanode(y) || yang_keyword_get(y) == Y_RPC || yang_keyword_get(y) == Y_CHOICE || yang_keyword_get(y) == Y_CASE || yang_keyword_get(y) == Y_INPUT || yang_keyword_get(y) == Y_OUTPUT || yang_keyword_get(y) == Y_NOTIFICATION) + /* * Prototypes */ diff --git a/lib/src/clixon_yang.c b/lib/src/clixon_yang.c index ae289858..d8cb5865 100644 --- a/lib/src/clixon_yang.c +++ b/lib/src/clixon_yang.c @@ -687,14 +687,13 @@ yang_find_datanode(yang_stmt *yn, yang_stmt *yspec; yang_stmt *ysmatch = NULL; char *name; - int i, j; - for (i=0; iys_len; i++){ - ys = yn->ys_stmt[i]; - if (ys->ys_keyword == Y_CHOICE){ /* Look for its children */ - for (j=0; jys_len; j++){ - yc = ys->ys_stmt[j]; - if (yc->ys_keyword == Y_CASE) /* Look for its children */ + ys = NULL; + while ((ys = yn_each(yn, ys)) != NULL){ + if (yang_keyword_get(ys) == Y_CHOICE){ /* Look for its children */ + yc = NULL; + while ((yc = yn_each(ys, yc)) != NULL){ + if (yang_keyword_get(yc) == Y_CASE) /* Look for its children */ ysmatch = yang_find_datanode(yc, argument); else if (yang_datanode(yc)){ @@ -726,8 +725,8 @@ yang_find_datanode(yang_stmt *yn, (yang_keyword_get(yn) == Y_MODULE || yang_keyword_get(yn) == Y_SUBMODULE)){ yspec = ys_spec(yn); - for (i=0; iys_len; i++){ - ys = yn->ys_stmt[i]; + ys = NULL; + while ((ys = yn_each(yn, ys)) != NULL){ if (yang_keyword_get(ys) == Y_INCLUDE){ name = yang_argument_get(ys); yc = yang_find_module_by_name(yspec, name); @@ -2083,12 +2082,15 @@ yang_apply(yang_stmt *yn, int yang_datanode(yang_stmt *ys) { - return (yang_keyword_get(ys) == Y_CONTAINER || - yang_keyword_get(ys) == Y_LEAF || - yang_keyword_get(ys) == Y_LIST || - yang_keyword_get(ys) == Y_LEAF_LIST || - yang_keyword_get(ys) == Y_ANYXML || - yang_keyword_get(ys) == Y_ANYDATA); + enum rfc_6020 keyw; + + keyw = yang_keyword_get(ys); + return (keyw == Y_CONTAINER || + keyw == Y_LEAF || + keyw == Y_LIST || + keyw == Y_LEAF_LIST || + keyw == Y_ANYXML || + keyw == Y_ANYDATA); } /*! All the work for schema_nodeid functions both absolute and descendant diff --git a/lib/src/clixon_yang_internal.h b/lib/src/clixon_yang_internal.h index 44753f06..74e4ccc1 100644 --- a/lib/src/clixon_yang_internal.h +++ b/lib/src/clixon_yang_internal.h @@ -90,21 +90,6 @@ struct yang_stmt{ int _ys_vector_i; /* internal use: yn_each */ }; -/* Yang data definition statement - * See RFC 7950 Sec 3: - * o data definition statement: A statement that defines new data - * nodes. One of "container", "leaf", "leaf-list", "list", "choice", - * "case", "augment", "uses", "anydata", and "anyxml". - */ -#define yang_datadefinition(y) (yang_datanode(y) || (y)->ys_keyword == Y_CHOICE || (y)->ys_keyword == Y_CASE || (y)->ys_keyword == Y_AUGMENT || (y)->ys_keyword == Y_USES) - -/* Yang schema node . - * See RFC 7950 Sec 3: - * o schema node: A node in the schema tree. One of action, container, - * leaf, leaf-list, list, choice, case, rpc, input, output, - * notification, anydata, and anyxml. - */ -#define yang_schemanode(y) (yang_datanode(y) || (y)->ys_keyword == Y_RPC || (y)->ys_keyword == Y_CHOICE || (y)->ys_keyword == Y_CASE || (y)->ys_keyword == Y_INPUT || (y)->ys_keyword == Y_OUTPUT || (y)->ys_keyword == Y_NOTIFICATION) #endif /* _CLIXON_YANG_INTERNAL_H_ */ diff --git a/lib/src/clixon_yang_parse_lib.c b/lib/src/clixon_yang_parse_lib.c index b78edbcf..e9a1d61a 100644 --- a/lib/src/clixon_yang_parse_lib.c +++ b/lib/src/clixon_yang_parse_lib.c @@ -835,7 +835,7 @@ yang_parse_recurse(clicon_handle h, /*! * @param[in] ys Yang statement * @param[in] dummy Necessary for called in yang_apply - * @see yang_apply_fn + * @see yang_applyfn_t */ static int ys_schemanode_check(yang_stmt *ys, @@ -906,6 +906,69 @@ ys_schemanode_check(yang_stmt *ys, return retval; } +/*! Check lists: non-config lists MUST have keys + * @param[in] h Clicon handle + * @param[in] ys Yang statement + * Verify the following rule: + * RFC 7950 7.8.2: The "key" statement, which MUST be present if the list represents + * configuration and MAY be present otherwise + * Unless CLICON_YANG_LIST_CHECK is false + * OR it is the "errors" rule of the ietf-restconf spec which seems to be a special case. + */ +static int +ys_list_check(clicon_handle h, + yang_stmt *ys) +{ + int retval = -1; + yang_stmt *ymod; + yang_stmt *yc = NULL; + enum rfc_6020 keyw; + + /* This node has config false */ + if (yang_config(ys) == 0) + return 0; + keyw = yang_keyword_get(ys); + /* Check if list and if keys do not exist */ + if (keyw == Y_LIST && + yang_find(ys, Y_KEY, NULL) == 0){ + ymod = ys_module(ys); +#if 1 + /* Except restconf error extension from sanity check, dont know why it has no keys */ + if (strcmp(yang_find_mynamespace(ys),"urn:ietf:params:xml:ns:yang:ietf-restconf")==0 && + strcmp(yang_argument_get(ys),"error") == 0) + ; + else +#endif + { + if (clicon_option_bool(h, "CLICON_YANG_LIST_CHECK")){ + clicon_log(LOG_ERR, "Error: LIST \"%s\" in module \"%s\" lacks key statement which MUST be present (See RFC 7950 Sec 7.8.2)", + yang_argument_get(ys), + yang_argument_get(ymod) + ); + + goto done; + } + else + clicon_log(LOG_WARNING, "Warning: LIST \"%s\" in module \"%s\" lacks key statement which MUST be present (See RFC 7950 Sec 7.8.2)", + yang_argument_get(ys), + yang_argument_get(ymod) + ); + } + } + /* Traverse subs */ + if (yang_schemanode(ys) || keyw == Y_MODULE || keyw == Y_SUBMODULE){ + yc = NULL; + while ((yc = yn_each(ys, yc)) != NULL){ + if (ys_list_check(h, yc) < 0) + goto done; + } + } + + retval = 0; + done: + return retval; +} + /*! Parse top yang module including all its sub-modules. Expand and populate yang tree * * Perform secondary actions after yang parsing. These actions cannot be made at @@ -986,10 +1049,15 @@ yang_parse_post(clicon_handle h, if (yang_apply(yspec->ys_stmt[i], -1, ys_populate2, (void*)h) < 0) goto done; - /* 8: sanity check of schemanode references, need more here */ - for (i=modnr; iys_stmt[i], -1, ys_schemanode_check, NULL) < 0) goto done; + /* Check list key values */ + if (ys_list_check(h, yspec->ys_stmt[i]) < 0) + goto done; + } retval = 0; done: return retval; diff --git a/test/test_yang_models.sh b/test/test_yang_models.sh index 2c1535a5..c723fb89 100755 --- a/test/test_yang_models.sh +++ b/test/test_yang_models.sh @@ -40,6 +40,7 @@ cat < $cfg $YANGMODELS/standard/ieee/draft/802 $YANGMODELS/standard/ieee/published/802.1 /usr/local/share/clixon + false /usr/local/lib/$APPNAME/clispec /usr/local/lib/$APPNAME/cli $APPNAME @@ -106,7 +107,7 @@ let i=0; for f in $files; do if [ -n "$(head -5 $f|grep '^ module')" ]; then new "$clixon_cli -1f $cfg -o CLICON_YANG_MAIN_FILE=$f -p $YANGMODELS/vendor/juniper/18.2/18.2R1/common -p $YANGMODELS/vendor/juniper/18.2/18.2R1/junos/conf show version" - expectfn "$clixon_cli -1f $cfg -o CLICON_YANG_MAIN_FILE=$f -p $YANGMODELS/vendor/juniper/18.2/18.2R1/common -p $YANGMODELS/vendor/juniper/18.2/18.2R1/junos/conf -o CLICON_CLI_GENMODEL=0 show version" 0 "$version." + expectfn "$clixon_cli -D $DBG -1f $cfg -o CLICON_YANG_MAIN_FILE=$f -p $YANGMODELS/vendor/juniper/18.2/18.2R1/common -p $YANGMODELS/vendor/juniper/18.2/18.2R1/junos/conf -o CLICON_CLI_GENMODEL=0 show version" 0 "$version." let i++; sleep 1 fi diff --git a/yang/clixon/clixon-config@2020-02-22.yang b/yang/clixon/clixon-config@2020-02-22.yang index 1ba45dfe..d02653bb 100644 --- a/yang/clixon/clixon-config@2020-02-22.yang +++ b/yang/clixon/clixon-config@2020-02-22.yang @@ -46,7 +46,8 @@ module clixon-config { Added: clixon-stats state for clixon XML and memory statistics. Added: CLICON_CLI_BUF_START and CLICON_CLI_BUF_THRESHOLD for quadratic and linear growth of CLIgen buffers (cbuf:s) - Added: CLICON_VALIDATE_STATE_XML for controling validation of user state XML"; + Added: CLICON_VALIDATE_STATE_XML for controling validation of user state XML + Added: CLICON_CLICON_YANG_LIST_CHECK to skip list key checks"; } revision 2019-09-11 { description @@ -302,6 +303,14 @@ module clixon-config { There is a 'good-enough' posix translation mode and a complete libxml2 mode"; } + leaf CLICON_YANG_LIST_CHECK { + type boolean; + default true; + description + "If false, skip Yang list check sanity checks from RFC 7950, Sec 7.8.2: + The 'key' statement, which MUST be present if the list represents configuration. + Some yang specs seem not to fulfil this."; + } leaf CLICON_BACKEND_DIR { type string; description