System-only config: Source of truth

Candidate, fix system-only in cache when locked or modified, re-read otherwise
Remove system-only from cache after commit

CHANGELOG.md

@@ -20,11 +20,11 @@ Expected: January 2025
 * New: [feature request: support xpath functions for strings](https://github.com/clicon/clixon/issues/556)
   * Added: re-match, substring, string, string-length, translate, substring-before, substring-after, starts-with
 * Added support for system-only-config data
-  * A mechanism to not store sensitive data in the datastore, instead use application callbacks to store the data in system state.
+  * Store sensitive data in the "system" instead of in datastores
   * New `CLICON_XMLDB_SYSTEM_ONLY_CONFIG` configuration option
   * New `system-only-config` extension
   * New `ca_system_only` backend callback for reading system-only data
-* New `clixon-config@2024-08-01.yang` revision
+* New `clixon-config@2024-11-01.yang` revision
   * Added: `CLICON_XMLDB_SYSTEM_ONLY_CONFIG`
 
 ### C/CLI-API changes on existing features

apps/backend/backend_client.c

@@ -187,14 +187,14 @@ release_all_dbs(clixon_handle h,
     char    **keys = NULL;
     size_t    klen;
     int       i;
-    uint32_t  iddb;
     db_elmnt *de;
 
-    if (clicon_option_bool(h, "CLICON_AUTOLOCK") &&
-        (iddb = xmldb_islocked(h, "candidate")) == id){
-        if (xmldb_copy(h, "running", "candidate") < 0)
-            goto done;
-        xmldb_modified_set(h, "candidate", 0); /* reset dirty bit */
+    if (xmldb_islocked(h, "candidate") == id){
+        if (clicon_option_bool(h, "CLICON_AUTOLOCK")){
+            if (xmldb_copy(h, "running", "candidate") < 0)
+                goto done;
+            xmldb_modified_set(h, "candidate", 0); /* reset dirty bit */
+        }
     }
     /* get all db:s */
     if (clicon_hash_keys(clicon_db_elmnt(h), &keys, &klen) < 0)
@@ -204,8 +204,6 @@ release_all_dbs(clixon_handle h,
         if ((de = clicon_db_elmnt_get(h, keys[i])) != NULL &&
             de->de_id == id){
             de->de_id = 0; /* unlock */
-
-
             clicon_db_elmnt_set(h, keys[i], de);
             if (clixon_plugin_lockdb_all(h, keys[i], 0, id) < 0)
                 goto done;
@@ -477,12 +475,13 @@ do_lock(clixon_handle h,
     /* 2) The target configuration is <candidate>, it has already been modified, and
      *    these changes have not been committed or rolled back.
      */
-    if (strcmp(db, "candidate") == 0 &&
-        xmldb_modified_get(h, db)){
-        if (netconf_lock_denied(cbret, "<session-id>0</session-id>",
-                                "Operation failed, candidate has already been modified and the changes have not been committed or rolled back (RFC 6241 7.5)") < 0)
-            goto done;
-        goto failed;
+    if (strcmp(db, "candidate") == 0) {
+        if (xmldb_exists(h, db) && xmldb_modified_get(h, db)){
+            if (netconf_lock_denied(cbret, "<session-id>0</session-id>",
+                                    "Operation failed, candidate has already been modified and the changes have not been committed or rolled back (RFC 6241 7.5)") < 0)
+                goto done;
+            goto failed;
+        }
     }
     /* 3) The target configuration is <running>, and another NETCONF
      *    session has an ongoing confirmed commit
@@ -500,6 +499,13 @@ do_lock(clixon_handle h,
     }
     if (xmldb_lock(h, db, id) < 0)
         goto done;
+    if (strcmp(db, "candidate") == 0) {
+        /* Add system-only config to candidate cache */
+        if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG")){
+            if (system_only_data_add(h, "candidate") < 0)
+                goto done;
+        }
+    }
     /* user callback */
     if (clixon_plugin_lockdb_all(h, db, 1, id) < 0)
         goto done;
@@ -586,6 +592,13 @@ from_client_edit_config(clixon_handle h,
         if (ret == 0)
             goto ok;
     }
+    if (strcmp(target, "candidate") == 0) {
+        /* Add system-only config to candidate cache */
+        if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG")){
+            if (system_only_data_add(h, "candidate") < 0)
+                goto done;
+        }
+    }
     if (xml_nsctx_node(xn, &nsc) < 0)
         goto done;
     /* Get prefix of netconf base namespace in the incoming message */
@@ -733,6 +746,7 @@ from_client_edit_config(clixon_handle h,
         goto done;
     }
     cprintf(cbret, "<rpc-reply xmlns=\"%s\"><ok", NETCONF_BASE_NAMESPACE);
+    // Set in text_modify
     if (clicon_data_get(h, "objectexisted", &val) == 0)
         cprintf(cbret, " %s:objectexisted=\"%s\" xmlns:%s=\"%s\"",
                 CLIXON_LIB_PREFIX, val,
@@ -820,7 +834,21 @@ from_client_copy_config(clixon_handle h,
             goto done;
         goto ok;
     }
-    xmldb_modified_set(h, target, 1); /* mark as dirty */
+
+    if (strcmp(target, "candidate") == 0){
+        xmldb_modified_set(h, target, 1); /* mark as dirty */
+        /* Add system-only config to candidate */
+        if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG")){
+            if (system_only_data_add(h, "candidate") < 0)
+                goto done;
+        }
+    }
+    /* Remove system-only-config data from destination cache */
+    if (strcmp(source, "candidate") == 0){
+        if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG")){
+            xmldb_clear(h, target);
+        }
+    }
     cprintf(cbret, "<rpc-reply xmlns=\"%s\"><ok/></rpc-reply>", NETCONF_BASE_NAMESPACE);
  ok:
     retval = 0;
@@ -974,6 +1002,10 @@ from_client_lock(clixon_handle h,
  * @param[in]  regarg  User argument given at rpc_callback_register()
  * @retval     0       OK
  * @retval    -1       Error
+ *
+ * RFC 6241, 8.3.5.2: To facilitate easy recovery, any outstanding changes are discarded when the
+ * lock is released, whether explicitly with the <unlock> operation or implicitly from session
+ * failure. (XXX This is not implemented)
  */
 static int
 from_client_unlock(clixon_handle h,

apps/backend/backend_commit.c

@@ -730,9 +730,10 @@ candidate_commit(clixon_handle  h,
      */
     if (xmldb_copy(h, db, "running") < 0)
         goto done;
-    /* Remove system-only-config data from destination */
+    /* Remove system-only-config data from destination cache */
     if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG")){
         xmldb_clear(h, "running");
+        xmldb_clear(h, db);
     }
     xmldb_modified_set(h, db, 0); /* reset dirty bit */
     /* Here pointers to old (source) tree are obsolete */
@@ -1126,3 +1127,40 @@ load_failsafe(clixon_handle h,
         cbuf_free(cbret);
     return retval;
 }
+
+/*! Add system-only data to db
+ *
+ * @param[in]  h   Clixon handle
+ * @param[in]  db  Datastore
+ * @retval     0   OK
+ * @retval    -1   Error
+ */
+int
+system_only_data_add(clixon_handle h,
+                     char         *db)
+{
+    int    retval = -1;
+    cxobj *x;
+
+    if ((x = xmldb_cache_get(h, db)) != NULL){
+        if (xmldb_system_only_config(h, "/", NULL, &x) < 0)
+            goto done;
+    }
+    else{ // XXX extra complexity
+        if ((x = xml_new(DATASTORE_TOP_SYMBOL, NULL, CX_ELMNT)) == NULL)
+            goto done;
+        xml_flag_set(x, XML_FLAG_TOP);
+        if (xmldb_system_only_config(h, "/", NULL, &x) < 0)
+            goto done;
+        if (xml_child_nr(x)){
+            db_elmnt *de;
+            if ((de = clicon_db_elmnt_get(h, db)) != NULL)
+                de->de_xml = x;
+        }
+        else
+            xml_free(x);
+    }
+    retval = 0;
+ done:
+    return retval;
+}

apps/backend/backend_main.c

@@ -1030,16 +1030,9 @@ main(int    argc,
     /* Initiate the shared candidate. */
     if (xmldb_copy(h, "running", "candidate") < 0)
         goto done;
-    /* Add system-only config to candidate */
-    if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG")){
-        cxobj *x;
-        if ((x = xmldb_cache_get(h, "candidate")) != NULL)
-            if (xmldb_system_only_config(h, "/", NULL, &x) < 0)
-                goto done;
-    }
+
     if (xmldb_modified_set(h, "candidate", 0) <0)
         goto done;
-
     /* Set startup status */
     if (clicon_startup_status_set(h, status) < 0)
         goto done;

apps/backend/clixon_backend_commit.h

@@ -73,11 +73,11 @@ int startup_commit(clixon_handle h, char *db, cbuf *cbret);
 int candidate_validate(clixon_handle h, char *db, cbuf *cbret);
 int candidate_commit(clixon_handle h, cxobj *xe, char *db, uint32_t myid,
                      validate_level vlev, cbuf *cbret);
-
 int from_client_commit(clixon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg);
 int from_client_discard_changes(clixon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg);
 int from_client_validate(clixon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg);
 int from_client_restart_one(clixon_handle h, clixon_plugin_t *cp, cbuf *cbret);
 int load_failsafe(clixon_handle h, char *phase);
+int system_only_data_add(clixon_handle h, char *db);
 
 #endif  /* _CLIXON_BACKEND_COMMIT_H_ */

example/main/example_backend.c

@@ -676,7 +676,7 @@ example_statefile(clixon_handle     h,
  * @retval     0   OK
  * @retval    -1   Error
  *
- * System-only config data as defined by _ is not written to datastore.
+ * System-only config data is not written to datastore.
  * Instead, in this ocmmit action, it is written to file _state_file
  * @see main_system_only_commit  callback for reading data
  * @note  Only single system-only config data supported

lib/src/clixon_datastore.c

@@ -428,6 +428,7 @@ xmldb_unlock(clixon_handle h,
         de->de_id = 0;
         memset(&de->de_tv, 0, sizeof(struct timeval));
         clicon_db_elmnt_set(h, db, de);
+
     }
     return 0;
 }
@@ -472,7 +473,7 @@ xmldb_unlock_all(clixon_handle h,
  *
  * @param[in] h   Clixon handle
  * @param[in] db  Database
- * @retval    >0  Session id of locker
+ * @retval   >0   Session id of locker
  * @retval    0   Not locked
  * @retval   -1   Error
  */
@@ -562,6 +563,9 @@ xmldb_clear(clixon_handle h,
             xml_free(xt);
             de->de_xml = NULL;
         }
+        de->de_modified = 0;
+        de->de_id = 0;
+        memset(&de->de_tv, 0, sizeof(struct timeval));
     }
     return 0;
 }
@@ -572,8 +576,8 @@ xmldb_clear(clixon_handle h,
  * @param[in]  db  Database
  * @retval     0   OK
  * @retval    -1   Error
-
- * @note  Datastore is not actually deleted so that a backend after dropping priviliges can re-create it
+ * @note Datastores / dirs are not actually deleted so that a backend after dropping priviliges
+ *       can re-create them
  */
 int
 xmldb_delete(clixon_handle h,
@@ -612,17 +616,11 @@ xmldb_delete(clixon_handle h,
             for (i = 0; i < ndp; i++){
                 cbuf_reset(cb);
                 cprintf(cb, "%s/%s", subdir, dp[i].d_name);
-                if (unlink(cbuf_get(cb)) < 0){
-                    clixon_err(OE_DB, errno, "unlink(%s)", cbuf_get(cb));
+                if (truncate(cbuf_get(cb), 0) < 0){
+                    clixon_err(OE_DB, errno, "truncate %s", filename);
                     goto done;
                 }
             }
-            if (rmdir(subdir) < 0){
-#if 0 /* Ignore this for now, there are some cornercases where this is problamatic, see confirmed-commit */
-                clixon_err(OE_DB, errno, "rmdir(%s)", subdir);
-                goto done;
-#endif
-            }
         }
     }
     retval = 0;

lib/src/clixon_datastore_read.c

@@ -886,10 +886,13 @@ xmldb_get_cache(clixon_handle     h,
         if (xml_apply(x1t, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset, (void*)(XML_FLAG_MARK|XML_FLAG_CHANGE)) < 0)
             goto done;
     }
-    if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG") &&
-        (strcmp(db, "candidate") != 0)) {
-        if (xmldb_system_only_config(h, xpath?xpath:"/", nsc, &x1t) < 0)
-            goto done;
+    /* Unless a modified/locked candidate, add system-only-config data */
+    if (strcmp(db, "candidate") != 0 ||
+        (xmldb_modified_get(h, db) == 0 &&
+         xmldb_islocked(h, db) == 0)){
+        if (clicon_option_bool(h, "CLICON_XMLDB_SYSTEM_ONLY_CONFIG"))
+            if (xmldb_system_only_config(h, xpath?xpath:"/", nsc, &x1t) < 0)
+                goto done;
     }
     /* If empty NACM config, then disable NACM if loaded
      */

lib/src/clixon_datastore_write.c

@@ -1239,7 +1239,15 @@ text_modify_top(clixon_handle       h,
     /* Special case top-level replace */
     else if (op == OP_REPLACE || op == OP_DELETE){
         if (createstr != NULL){
-            if (xml_child_nr_type(x0t, CX_ELMNT)) /* base tree not empty */
+            x0c = NULL;
+            /* Specialization of xml_default_nopresence for skiptop and mode=0 */
+            while ((x0c = xml_child_each(x0t, x0c, CX_ELMNT)) != NULL) {
+                if ((ret = xml_default_nopresence(x0c, 0, 0)) < 0)
+                    goto done;
+                if (ret == 0)
+                    break;
+            }
+            if (x0c != NULL)
                 clicon_data_set(h, "objectexisted", "true");
             else
                 clicon_data_set(h, "objectexisted", "false");
@@ -1421,6 +1429,12 @@ xmldb_put(clixon_handle       h,
             goto done;
         if (ret == 0)
             goto fail;
+        /* Add default global values (see also xmldb_populate) */
+        if (xml_global_defaults(h, x0, nsc, "/", yspec, 0) < 0)
+            goto done;
+        /* Add default recursive values */
+        if (xml_default_recurse(x0, 0, 0) < 0)
+            goto done;
     }
     if (strcmp(xml_name(x0), DATASTORE_TOP_SYMBOL) !=0 ||
         xml_flag(x0, XML_FLAG_TOP) == 0){
@@ -1461,7 +1475,7 @@ xmldb_put(clixon_handle       h,
      */
     if (xml_default_nopresence(x0, 3, XML_FLAG_ADD|XML_FLAG_DEL) < 0)
         goto done;
-    /* Complete defaults in incoming x1
+    /* Complete defaults
      */
     if (xml_global_defaults(h, x0, nsc, "/", yspec, 0) < 0)
         goto done;

test/test_autocli_hide.sh

@@ -133,6 +133,7 @@ EOF
     new "check datastore using netconf"
     expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><get-config><source><candidate/></source><filter type=\"xpath\" select=\"/ex:table/ex:parameter[ex:name='x']\" xmlns:ex=\"urn:example:clixon\" /></get-config></rpc>" "" "<rpc-reply $DEFAULTNS><data>$XML</data></rpc-reply>"
 
+    sudo chmod a+r $dir/candidate_db
     new "check datastore direct access"
     expectpart "$($clixon_util_datastore -d candidate -b $dir -y $fyang -Y ${YANG_INSTALLDIR} -Y $dir get /)" 0 "$XML"