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`

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
+* 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
+* 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

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.

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
 

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)
 

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

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
  */

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; i<yn->ys_len; i++){
+    ys = NULL;
-	ys = yn->ys_stmt[i];
+    while ((ys = yn_each(yn, ys)) != NULL){
-	if (ys->ys_keyword == Y_CHOICE){ /* Look for its children */
+	if (yang_keyword_get(ys) == Y_CHOICE){ /* Look for its children */
-	    for (j=0; j<ys->ys_len; j++){
+	    yc = NULL;
-		yc = ys->ys_stmt[j];
+	    while ((yc = yn_each(ys, yc)) != NULL){
-		if (yc->ys_keyword == Y_CASE) /* Look for its children */
+		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; i<yn->ys_len; i++){
+	ys = NULL;
-	    ys = yn->ys_stmt[i];
+	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 ||
+    enum rfc_6020 keyw;
-	     yang_keyword_get(ys) == Y_LEAF ||
+
-	     yang_keyword_get(ys) == Y_LIST ||
+    keyw = yang_keyword_get(ys);
-	     yang_keyword_get(ys) == Y_LEAF_LIST ||
+    return (keyw == Y_CONTAINER ||
-	     yang_keyword_get(ys) == Y_ANYXML ||
+	    keyw == Y_LEAF ||
-	     yang_keyword_get(ys) == Y_ANYDATA);
+	    keyw == Y_LIST ||
+	    keyw == Y_LEAF_LIST ||
+	    keyw == Y_ANYXML ||
+	    keyw == Y_ANYDATA);
 }
 
 /*! All the work for schema_nodeid functions both absolute and descendant

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_ */
 

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 */
+    /* 8: sanity checks of expanded yangs need more here */
-    for (i=modnr; i<yang_len_get(yspec); i++)
+    for (i=modnr; i<yang_len_get(yspec); i++){
+	/* Check schemanode references */
 	if (yang_apply(yspec->ys_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;

test/test_yang_models.sh

@@ -40,6 +40,7 @@ cat <<EOF > $cfg
   <CLICON_YANG_DIR>$YANGMODELS/standard/ieee/draft/802</CLICON_YANG_DIR> 
   <CLICON_YANG_DIR>$YANGMODELS/standard/ieee/published/802.1</CLICON_YANG_DIR> 
   <CLICON_YANG_DIR>/usr/local/share/clixon</CLICON_YANG_DIR>
+  <CLICON_YANG_LIST_CHECK>false</CLICON_YANG_LIST_CHECK>
   <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
   <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
   <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>
@@ -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

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