Confirmed-commit handle drop privileges

Create and drop priv of rollback datastore on startup
Reverted xmldb to truncate instead of deleting datastores due to privileges drop

CHANGELOG.md

@@ -53,7 +53,6 @@ Expected: End of 2022
     * See [example_cli.cli](https://github.com/clicon/clixon/blob/master/example/main/example_cli.cli)
   * See [Netconf Confirmed Commit Capability](https://github.com/clicon/clixon/issues/255)
   * Known issues
-    * Backend privileges drop
     * Lock check, see RFC 6241 7.5
 
 ### API changes on existing protocol/config features

apps/backend/backend_main.c

@@ -280,7 +280,8 @@ xmldb_drop_priv(clicon_handle h,
  */
 static int
 check_drop_priv(clicon_handle h,
-		gid_t         gid)
+		gid_t         gid,
+    		yang_stmt    *yspec)
 {
     int              retval = -1;
     uid_t            uid;
@@ -288,7 +289,6 @@ check_drop_priv(clicon_handle h,
     enum priv_mode_t priv_mode = PM_NONE;
     char            *backend_user = NULL;
 
-
     /* Get privileges mode (for dropping privileges) */
     if ((priv_mode = clicon_backend_privileges_mode(h)) == PM_NONE)
 	goto ok;
@@ -325,12 +325,20 @@ check_drop_priv(clicon_handle h,
 	    goto done;
     if (xmldb_drop_priv(h, "candidate", newuid, gid) < 0)
 	goto done;
+    if (if_feature(yspec, "ietf-netconf", "startup")) {
 	if (xmldb_exists(h, "startup") != 1)
 	    if (xmldb_create(h, "startup") < 0)
 		goto done;
 	if (xmldb_drop_priv(h, "startup", newuid, gid) < 0)
 	    goto done;
-
+    }
+    if (if_feature(yspec, "ietf-netconf", "confirmed-commit")) {
+	if (xmldb_exists(h, "rollback") != 1)
+	    if (xmldb_create(h, "rollback") < 0)
+		goto done;
+	if (xmldb_drop_priv(h, "rollback", newuid, gid) < 0)
+	    goto done;
+    }
     if (setgid(gid) == -1) {
 	clicon_err(OE_DAEMON, errno, "setgid %d", gid);
 	goto done;
@@ -1041,7 +1049,7 @@ main(int    argc,
 	clicon_option_dump(h, dbg);
     /* Depending on configure setting, privileges may be dropped here after
      * initializations */
-    if (check_drop_priv(h, gid) < 0)
+    if (check_drop_priv(h, gid, yspec) < 0)
 	goto done;
 
     /* Start session-id for clients */

lib/src/clixon_datastore.c

@@ -199,6 +199,7 @@ xmldb_copy(clicon_handle h,
     cxobj              *x1 = NULL;  /* from */
     cxobj              *x2 = NULL;  /* to */
 
+    clicon_debug(1, "%s %s %s", __FUNCTION__, from, to);
     /* XXX lock */
     if (clicon_datastore_cache(h) != DATASTORE_NOCACHE){
 	/* Copy in-memory cache */
@@ -252,7 +253,6 @@ xmldb_copy(clicon_handle h,
     if (tofile)
 	free(tofile);
     return retval;
-
 }
 
 /*! Lock database
@@ -350,12 +350,13 @@ xmldb_islocked(clicon_handle h,
     return de->de_id;
 }
 
-/*! Check if db exists 
+/*! Check if db exists or is empty
  * @param[in]  h   Clicon handle
  * @param[in]  db  Database
  * @retval -1  Error
  * @retval  0  No it does not exist
  * @retval  1  Yes it exists
+ * @note  An empty datastore is treated as not existent so that a backend after dropping priviliges can re-create it
  */
 int 
 xmldb_exists(clicon_handle h, 
@@ -365,12 +366,18 @@ xmldb_exists(clicon_handle h,
     char               *filename = NULL;
     struct stat         sb;
 
+    clicon_debug(2, "%s %s", __FUNCTION__, db);
     if (xmldb_db2file(h, db, &filename) < 0)
 	goto done;
     if (lstat(filename, &sb) < 0)
 	retval = 0;
+
+    else{
+	if (sb.st_size == 0)
+	    retval = 0;
 	else
 	    retval = 1;
+    }
  done:
     if (filename)
 	free(filename);
@@ -404,6 +411,7 @@ xmldb_clear(clicon_handle h,
  * @param[in]  db  Database
  * @retval -1  Error
  * @retval  0  OK
+ * @note  Datastore is not actually deleted so that a backend after dropping priviliges can re-create it
  */
 int 
 xmldb_delete(clicon_handle h, 
@@ -413,22 +421,14 @@ xmldb_delete(clicon_handle h,
     char               *filename = NULL;
     struct stat         sb;
     
+    clicon_debug(2, "%s %s", __FUNCTION__, db);
     if (xmldb_clear(h, db) < 0)
 	goto done;
     if (xmldb_db2file(h, db, &filename) < 0)
 	goto done;
     if (lstat(filename, &sb) == 0)
-        // TODO this had been changed from unlink to truncate some time ago, likely related to dropping privileges.
+	if (truncate(filename, 0) < 0){
-        // It was changed back for confirmed-commit, and test_confirmed_commit.sh drops privileges.
+	    clicon_err(OE_DB, errno, "truncate %s", filename);
-        // as the presence of the rollback_db at startup triggers loading of the rollback rather than the startup
-        // configuration.  It might not be sufficient to check for a truncated file.  Needs more review, switching back
-        // to unlink temporarily.
-//	if (truncate(filename, 0) < 0){
-//	    clicon_err(OE_DB, errno, "truncate %s", filename);
-//	    goto done;
-//	}
-        if (unlink(filename) < 0) {
-            clicon_err(OE_UNIX, errno, "unlink %s: %s", filename, strerror(errno));
 	    goto done;
 	}
     retval = 0;
@@ -454,6 +454,7 @@ xmldb_create(clicon_handle h,
     db_elmnt           *de = NULL;
     cxobj              *xt = NULL;
 
+    clicon_debug(2, "%s %s", __FUNCTION__, db);
     if ((de = clicon_db_elmnt_get(h, db)) != NULL){
 	if ((xt = de->de_xml) != NULL){
 	    xml_free(xt);
@@ -607,7 +608,7 @@ xmldb_print(clicon_handle h,
 /*! Rename an XML database
  * @param[in]  h        Clicon handle
  * @param[in]  db       Database name
- * @param[in]  newdb    New Database name; if NULL, then same as new
+ * @param[in]  newdb    New Database name; if NULL, then same as old
  * @param[in]  suffix   Suffix to append to new database name
  * @retval    -1        Error
  * @retval     0        OK
@@ -619,48 +620,32 @@ xmldb_rename(clicon_handle h,
              const char    *newdb,
              const char    *suffix)
 {
+    int    retval = -1;
     char  *old;
     char  *fname = NULL;
-    int retval = -1;
+    cbuf  *cb = NULL;
 
-    if ((xmldb_db2file(h, db, &old)) < 0) {
+    if ((xmldb_db2file(h, db, &old)) < 0)
         goto done;
-    };
+    if (newdb == NULL && suffix == NULL)        // no-op
-
-    if (newdb == NULL && suffix == NULL)
-        // no-op
         goto done;
-
+    if ((cb = cbuf_new()) == NULL){
-    newdb  = newdb == NULL ? old : newdb;
+	clicon_err(OE_XML, errno, "cbuf_new");
-    suffix = suffix == NULL ? "" : suffix;
-
-    size_t size = strlen(newdb) + strlen(suffix);
-
-    if ((fname = malloc(size + 1)) == NULL) {
-        clicon_err(OE_UNIX, errno, "malloc: %s", strerror(errno));
 	goto done;
-    };
+    }
-
+    cprintf(cb, "%s", newdb == NULL ? old : newdb);
-    int actual = 0;
+    if (suffix)
-    if ((actual = snprintf(fname, size, "%s%s", newdb, suffix)) < size) {
+	cprintf(cb, "%s", suffix);
-        clicon_err(OE_UNIX, 0, "snprintf wrote fewer bytes (%d) than requested (%zu)", actual, size);
+    fname = cbuf_get(cb);
-        goto done;
-    };
-
     if ((rename(old, fname)) < 0) {
         clicon_err(OE_UNIX, errno, "rename: %s", strerror(errno));
         goto done;
     };
-
-
     retval = 0;
-
  done:
+    if (cb)
+	cbuf_free(cb);
     if (old)
         free(old);
-
-    if (fname)
-        free(fname);
-
     return retval;
 }

test/test_confirmed_commit.sh

@@ -15,7 +15,7 @@ tmp=$dir/tmp.x
 fyang=$dir/clixon-example.yang
 
 # Backend user for priv drop, otherwise root
-USER=root #${BUSER}
+USER=${BUSER}
 
 # Define default restconfig config: RESTCONFIG
 RESTCONFIG=$(restconf_config none false)
@@ -125,14 +125,13 @@ CONFIGBPLUSC="<table xmlns=\"urn:example:clixon\"><parameter><name>eth0</name></
 FAILSAFE_CFG="<table xmlns=\"urn:example:clixon\"><parameter><name>eth99</name></parameter></table>"
 
 new "test params: -f $cfg"
+
 # Bring your own backend
 if [ $BE -ne 0 ]; then
     # kill old backend (if any)
     new "kill old backend"
-    sudo clixon_backend -zf $cfg
+    stop_backend -f $cfg
-    if [ $? -ne 0 ]; then
+
-	err
-    fi
     new "start backend  -s init -f $cfg"
     start_backend -s init -f $cfg
 fi
@@ -229,32 +228,52 @@ commit ""
 edit_config "candidate" "$CONFIGC"
 commit "<persist>abcdefg</persist><confirmed/>"
 assert_config_equals "running" "$CONFIGBPLUSC"
+
+new "kill old backend"
 stop_backend -f $cfg                                            # kill backend and restart
+
+new "Check $ROLLBACK_PATH"
 [ -f "$ROLLBACK_PATH" ] || err "rollback_db doesn't exist!"     # assert rollback_db exists
+
+new "start backend -s running -f $cfg"
 start_backend -s running -f $cfg
+
+new "wait backend"
 wait_backend
+
 assert_config_equals "running" "$CONFIGB"
+
+new "Check $ROLLBACK_PATH removed"
 [ -f "ROLLBACK_PATH" ] && err "rollback_db still exists!"       # assert rollback_db doesn't exist
 
+new "kill old backend"
 stop_backend -f $cfg
+
+new "start backend -s init -f $cfg"
 start_backend -s init -f $cfg
 
 ################################################################################
 new "backend loads failsafe at startup if rollback present but cannot be loaded"
 
-if [ ${valgrindtest} -eq 2 ]; then # backend valgrind
+new "wait backend"
-    sleep 3
+wait_backend
-fi
+
 reset
 
 sudo tee "$FAILSAFE_PATH" > /dev/null << EOF                    # create a failsafe database
 <config>$FAILSAFE_CFG</config>
 EOF
 edit_config "candidate" "$CONFIGC"
+
 commit "<persist>foobar</persist><confirmed/>"
+
 assert_config_equals "running" "$CONFIGC"
+
+new "kill old backend"
 stop_backend -f $cfg                                            # kill the backend
+
 sudo rm $ROLLBACK_PATH                                          # modify rollback_db so it won't commit successfully
+
 sudo tee "$ROLLBACK_PATH" > /dev/null << EOF
 <foo>
   <bar>
@@ -262,12 +281,22 @@ sudo tee "$ROLLBACK_PATH" > /dev/null << EOF
     </bar>
 </foo>
 EOF
+
+new "start backend -s running -f $cfg"
 start_backend -s running -f $cfg
+
+new "wait backend"
 wait_backend
+
 assert_config_equals "running" "$FAILSAFE_CFG"
 
+new "kill old backend"
 stop_backend -f $cfg
+
+new "start backend -s init -f $cfg"
 start_backend -s init -f $cfg -lf/tmp/clixon.log -D1
+
+new "wait backend"
 wait_backend
 
 ################################################################################