NACM read default rule did not work properly if nacm was enabled AND no group\

s were defined

CHANGELOG.md

@@ -108,6 +108,7 @@
 * Added libgen.h for baseline()
 	
 ### Corrected Bugs
+* NACM read default rule did not work properly if nacm was enabled AND no groups were defined 
 * Re-inserted `cli_output_reset` for what was erroneuos thought to be an obsolete function
   * See in 3.9.0 minro changes: Replaced all calls to (obsolete) `cli_output` with `fprintf`
 * Allowed Yang extended Xpath functions (syntax only):

lib/src/clixon_nacm.c

@@ -567,22 +567,27 @@ nacm_datanode_read(cxobj  *xt,
     /* User's group */
     if (xpath_vec(xnacm, "groups/group[user-name='%s']", &gvec, &glen, username) < 0)
 	goto done;
-    /* 4. If no groups are found, continue with step 9. */
+    /* 4. If no groups are found (glen=0), continue and check read-default 
-    if (glen == 0)
+          in step 11. */
-	goto step9;
     /* 5. Process all rule-list entries, in the order they appear in the
         configuration.  If a rule-list's "group" leaf-list does not
         match any of the user's groups, proceed to the next rule-list
         entry. */
     if (xpath_vec(xnacm, "rule-list", &rlistvec, &rlistlen) < 0)
 	goto done;
+    /* read-default has default permit so should never be NULL */
+    if ((read_default = xml_find_body(xnacm, "read-default")) == NULL){
+	clicon_err(OE_XML, EINVAL, "No nacm read-default rule");
+	goto done;
+    }
     for (i=0; i<xrlen; i++){     /* Loop through requested nodes */
 	xr = xrvec[i]; /* requested node XR */
 	/* Loop through rule-list (steps 5,6,7) to find match of requested node
 	 */
 	xrule = NULL;
-	if (nacm_data_read_xr(xt, xr, gvec, glen, rlistvec, rlistlen,
+	/* Skip if no groups */
-			      &xrule) < 0) 
+	if (glen && nacm_data_read_xr(xt, xr, gvec, glen, rlistvec, rlistlen,
+				      &xrule) < 0) 
 	    goto done;
 	if (xrule){ /* xrule match requested node xr */
 	    if ((action = xml_find_body(xrule, "action")) == NULL)
@@ -601,8 +606,7 @@ nacm_datanode_read(cxobj  *xt,
         to "permit", then include the requested data node in the reply;
         otherwise, do not include the requested data node or any of its
         descendants in the reply.*/
-	    read_default = xml_find_body(xnacm, "read-default");
+	    if (strcmp(read_default, "deny")==0)
-	    if (read_default == NULL || strcmp(read_default, "deny")==0)
 		if (xml_purge(xr) < 0)
 		    goto done;
 	}
@@ -668,7 +672,7 @@ nacm_datanode_write(cxobj           *xt,
     cxobj  *xrule;
     int     match = 0;
     char   *action;
-    char   *write_default;
+    char   *write_default = NULL;
     
     if (xnacm == NULL)
 	goto permit;
@@ -747,8 +751,12 @@ nacm_datanode_write(cxobj           *xt,
     /*12.  For a "write" access operation, if the "write-default" leaf is
         set to "permit", then permit the data node access request;
         otherwise, deny the request.*/
-    write_default = xml_find_body(xnacm, "write-default");
+    /* write-default has default permit so should never be NULL */
-    if (write_default == NULL  || strcmp(write_default, "permit") != 0){
+    if ((write_default = xml_find_body(xnacm, "write-default")) == NULL){
+	clicon_err(OE_XML, EINVAL, "No nacm write-default rule");
+	goto done;
+    }
+    if (strcmp(write_default, "permit") != 0){
 	if (netconf_access_denied(cbret, "application", "default deny") < 0)
 	    goto done;
 	goto deny;

test/test_nacm_default.sh

@@ -0,0 +1,193 @@
+#!/bin/bash
+# Basic NACM default rulw without any groups
+# Start from startup db
+
+# Magic line must be first in script (see README.md)
+s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
+
+APPNAME=example
+
+cfg=$dir/conf_yang.xml
+fyang=$dir/nacm-example.yang
+
+cat <<EOF > $cfg
+<clixon-config xmlns="http://clicon.org/config">
+  <CLICON_CONFIGFILE>$cfg</CLICON_CONFIGFILE>
+  <CLICON_YANG_DIR>/usr/local/share/clixon</CLICON_YANG_DIR>
+  <CLICON_YANG_DIR>$IETFRFC</CLICON_YANG_DIR>
+  <CLICON_YANG_MAIN_FILE>$fyang</CLICON_YANG_MAIN_FILE>
+  <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
+  <CLICON_RESTCONF_DIR>/usr/local/lib/$APPNAME/restconf</CLICON_RESTCONF_DIR>
+  <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
+  <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>
+  <CLICON_SOCK>/usr/local/var/$APPNAME/$APPNAME.sock</CLICON_SOCK>
+  <CLICON_BACKEND_DIR>/usr/local/lib/$APPNAME/backend</CLICON_BACKEND_DIR>
+  <CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
+  <CLICON_CLI_GENMODEL_COMPLETION>1</CLICON_CLI_GENMODEL_COMPLETION>
+  <CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
+  <CLICON_XMLDB_PLUGIN>/usr/local/lib/xmldb/text.so</CLICON_XMLDB_PLUGIN>
+  <CLICON_RESTCONF_PRETTY>false</CLICON_RESTCONF_PRETTY>
+  <CLICON_NACM_MODE>internal</CLICON_NACM_MODE>
+</clixon-config>
+EOF
+
+cat <<EOF > $fyang
+module nacm-example{
+  yang-version 1.1;
+  namespace "urn:example:nacm";
+  prefix nacm;
+  import clixon-example {
+	prefix ex;
+  }
+  import ietf-netconf-acm {
+	prefix nacm;
+  }
+  leaf x{
+    type int32;
+    description "something to edit";
+  }
+}
+EOF
+
+# 
+# startup db with default values: 
+# 1: enable-nacm (true|false)
+# 2: read-default (deny|permit)
+# 3: write-default (deny|permit)
+# 4: exec-defautl (deny|permit)
+# 5: expected return value of test1
+# 6: expected return value of test2
+# 7: expected return value of test3
+testrun(){
+    enablenacm=$1
+    readdefault=$2
+    writedefault=$3
+    execdefault=$4
+    ret1=$5
+    ret2=$6
+    ret3=$7
+
+    # NACM in startup
+    sudo tee $dir/startup_db > /dev/null << EOF
+    <config>
+   <nacm xmlns="urn:ietf:params:xml:ns:yang:ietf-netconf-acm">
+     <enable-nacm>${enablenacm}</enable-nacm>
+     <read-default>${readdefault}</read-default>
+     <write-default>${writedefault}</write-default>
+     <exec-default>${execdefault}</exec-default>
+     <enable-external-groups>true</enable-external-groups>
+   </nacm>
+   <x xmlns="urn:example:nacm">42</x>
+   </config>
+EOF
+    if [ $BE -ne 0 ]; then     # Bring your own backend
+	new "kill old backend"
+	sudo clixon_backend -zf $cfg
+	if [ $? -ne 0 ]; then
+	    err
+	fi
+	new "start backend -s init -f $cfg"
+	start_backend -s startup -f $cfg
+    else
+	new "Restart backend as eg follows: -Ff $cfg -s startup"
+	sleep $BETIMEOUT
+    fi
+    
+    new "kill old restconf daemon"
+    sudo pkill -u www-data -f "/www-data/clixon_restconf"
+
+    sleep 1
+    new "start restconf daemon (-a is enable basic authentication)"
+    start_restconf -f $cfg -- -a
+
+    new "waiting"
+    sleep $RCWAIT
+
+    #----------- First get
+    case "$ret1" in
+	0) ret='{"nacm-example:x": 42}
+
'
+	;;
+	1) ret='{"ietf-restconf:errors" : {"error": {"error-type": "application","error-tag": "access-denied","error-severity": "error","error-message": "default deny"}}}
'
+	   ;;
+	2) ret='null
+
'
+	;;
+    esac
+    new "get startup 42"
+    expecteq "$(curl -u guest:bar -sS -X GET http://localhost/restconf/data/nacm-example:x)" 0 "$ret"
+
+    #----------- Then edit
+    case "$ret2" in
+	0) ret=''
+	;;
+	1) ret='{"ietf-restconf:errors" : {"error": {"error-type": "application","error-tag": "access-denied","error-severity": "error","error-message": "default deny"}}}
'
+	;;
+    esac
+    new "edit new 99"
+    expecteq "$(curl -u guest:bar -sS -X PUT -d '{"nacm-example:x": 99}' http://localhost/restconf/data/nacm-example:x)" 0 "$ret"
+
+    #----------- Then second get
+    case "$ret3" in
+	0) ret='{"nacm-example:x": 99}
+
'
+	;;
+	1) ret='{"ietf-restconf:errors" : {"error": {"error-type": "application","error-tag": "access-denied","error-severity": "error","error-message": "default deny"}}}
'
+        ;;
+	2) ret='null
+
'
+	   ;;
+	3) ret='{"nacm-example:x": 42}
+
'
+    esac
+    new "get 99"
+    expecteq "$(curl -u guest:bar -sS -X GET http://localhost/restconf/data/nacm-example:x)" 0 "$ret"
+    
+    new "Kill restconf daemon"
+    stop_restconf
+    
+    if [ $BE -ne 0 ]; then     # Bring your own backend
+	new "Kill backend"
+	# Check if premature kill
+	pid=`pgrep -u root -f clixon_backend`
+	if [ -z "$pid" ]; then
+	    err "backend already dead"
+	fi
+	# kill backend
+	stop_backend -f $cfg
+    fi
+} # testrun
+
+# Run a lot of tests with different settings of default read/write/exec
+new "nacm enabled and all defaults permit"
+testrun true permit permit permit 0 0 0
+
+new "nacm disabled and all defaults permit"
+testrun false permit permit permit 0 0 0
+
+new "nacm disabled and all defaults deny"
+testrun false deny deny deny 0 0 0
+
+new "nacm enabled, all defaults deny (expect fail)"
+testrun true deny deny deny 1 1 1
+
+new "nacm enabled, exec default deny - read permit (expect fail)"
+testrun true permit deny deny 1 1 1
+
+new "nacm enabled, exec default deny - write permit (expect fail)"
+testrun true deny permit deny 1 1 1
+
+new "nacm enabled, exec default deny read/write permit (expect fail)"
+testrun true permit permit deny 1 1 1
+
+new "nacm enabled, exec default permit, all others deny (expect fail)"
+testrun true deny deny permit 2 1 2
+
+new "nacm enabled, exec default permit, read permit (expect fail)"
+testrun true permit deny permit 0 1 3
+
+new "nacm enabled, exec default permit, write permit (expect fail)"
+testrun true deny permit permit 2 0 2
+
+
+rm -rf $dir