diff --git a/CHANGELOG.md b/CHANGELOG.md index 947a26de..a9216baf 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index d6f7f993..dbe5680b 100644 --- a/apps/backend/backend_client.c +++ b/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 , 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, "0", - "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, "0", + "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 , 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, "", 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 operation or implicitly from session + * failure. (XXX This is not implemented) */ static int from_client_unlock(clixon_handle h, diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index 87704db8..fb4d2fa0 100644 --- a/apps/backend/backend_commit.c +++ b/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; +} diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c index 6245b706..b00fe2e4 100644 --- a/apps/backend/backend_main.c +++ b/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; diff --git a/apps/backend/clixon_backend_commit.h b/apps/backend/clixon_backend_commit.h index 8e053db1..b81c36af 100644 --- a/apps/backend/clixon_backend_commit.h +++ b/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_ */ diff --git a/example/main/example_backend.c b/example/main/example_backend.c index d853280f..7ad1241e 100644 --- a/example/main/example_backend.c +++ b/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 diff --git a/lib/src/clixon_datastore.c b/lib/src/clixon_datastore.c index ad6e06c4..2d85e7f7 100644 --- a/lib/src/clixon_datastore.c +++ b/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; diff --git a/lib/src/clixon_datastore_read.c b/lib/src/clixon_datastore_read.c index 2ff81357..e12e1de0 100644 --- a/lib/src/clixon_datastore_read.c +++ b/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 */ diff --git a/lib/src/clixon_datastore_write.c b/lib/src/clixon_datastore_write.c index 7286441b..fe0950b3 100644 --- a/lib/src/clixon_datastore_write.c +++ b/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; diff --git a/test/test_autocli_hide.sh b/test/test_autocli_hide.sh index dbb9a874..2c1a2273 100755 --- a/test/test_autocli_hide.sh +++ b/test/test_autocli_hide.sh @@ -133,6 +133,7 @@ EOF new "check datastore using netconf" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "$XML" + 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" diff --git a/test/test_datastore_system_only.sh b/test/test_datastore_system_only.sh index 43fef970..bf8a56ca 100755 --- a/test/test_datastore_system_only.sh +++ b/test/test_datastore_system_only.sh @@ -276,6 +276,56 @@ check_db running true xml new "Get mydata from running" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "amydataotherdata" +new "Get mydata from candidate" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "amydataotherdata" + +new "Source-of-truth: modify system-only" +sudo chmod 666 $dir/system-only.xml +cat < $dir/system-only.xml + + + + a + CHANGED + + + +EOF + +new "Get mydata from candidate again" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "aCHANGEDotherdata" + +new "Restore original" +cp $dir/y_db $dir/system-only.xml + +new "Get mydata from candidate again" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "amydataotherdata" + +new "Source-of-truth: modify system-only, then edit" +cat < $dir/system-only.xml + + + + a + CHANGED + + + +EOF + +new "Add normal data" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "aotherdata2" "" + +new "Get mydata from candidate expect CHANGED" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "aCHANGEDotherdata2" + +new "Restore original" +cp $dir/y_db $dir/system-only.xml + +new "Discard" +new "netconf discard-changes" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" + new "Remove mydata" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "amydatanone" "" @@ -419,7 +469,7 @@ fi new "wait restconf" wait_restconf -new "Add mydata" +new "Add system-only data" expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" -d '{"clixon-standard:store":{"keys":{"key":[{"name":"a","system-only-data":"mydata"}]}}}' $RCPROTO://localhost/restconf/data)" 0 "HTTP/$HVER 201" new "Add normal data" diff --git a/test/test_lock_auto.sh b/test/test_lock_auto.sh index ba40dab4..d826a321 100755 --- a/test/test_lock_auto.sh +++ b/test/test_lock_auto.sh @@ -97,30 +97,9 @@ wait_backend # clixon_cli < $dir/cli1 & # echo "show devices" > $dir/cli1 -if false; then - mkfifo $dir/cli1 -# cat > $dir/cli1 & - $clixon_cli -f $cfg < $dir/cli1 & - new "cli 1st edit async" - echo "set table parameter x value a" > $dir/cli1 - jobs -l % - PIDS=($(jobs -l % | cut -c 6- | awk '{print $1}')) -# echo "PIDS:$PIDS" - - new "cli 2nd edit expect fail" - expectpart "$($clixon_cli -1f $cfg set table parameter y value b 2>&1)" 255 "lock-denied" "lock is already held" - - kill ${PIDS[0]} # kill the while loop above to close STDIN on 1st - wait - - new "cli 3rd edit expect ok" -expectpart "$($clixon_cli -1f $cfg set table parameter z value c)" 0 "^$" -else new "cli 1st edit async" sleep 60 | expectpart "$($clixon_cli -f $cfg set table parameter x value a)" 0 "" & -if [ $valgrindtest -eq 1 ]; then - sleep 1 -fi +sleep 1 PIDS=($(jobs -l % | cut -c 6- | awk '{print $1}')) new "cli 2nd edit expect fail" @@ -152,7 +131,7 @@ PIDS=($(jobs -l % | cut -c 6- | awk '{print $1}')) new "cli edit 2nd expected ok" expectpart "$($clixon_cli -1f $cfg set table parameter x value a)" 0 "^$" -fi + if [ $BE -ne 0 ]; then new "Kill backend" # Check if premature kill diff --git a/test/test_netconf.sh b/test/test_netconf.sh index 727653b3..23e4076f 100755 --- a/test/test_netconf.sh +++ b/test/test_netconf.sh @@ -377,9 +377,7 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "]]>]]>") -| $clixon_netconf -qf $cfg >> /dev/null & -if [ $valgrindtest -eq 1 ]; then - sleep 1 -fi +sleep 1 PIDS=($(jobs -l % | cut -c 6- | awk '{print $1}')) new "try commit should fail" @@ -415,7 +413,7 @@ expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" " new "netconf lock" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" -new "copy startup to candidate" +new "copy candidate to startup" expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" new "copy startup to candidate using prefix xx" diff --git a/test/test_perf_mem.sh b/test/test_perf_mem.sh index d34449cc..2707f9d6 100755 --- a/test/test_perf_mem.sh +++ b/test/test_perf_mem.sh @@ -110,7 +110,8 @@ function testrun(){ echo -n " /proc/$pid/statm: " cat /proc/$pid/statm|awk '{print $1*4/1000 "M"}' fi - for db in running candidate startup; do + dbs="running candidate startup"; + for db in $dbs; do echo "$db" resdb0=$(echo "$res" | $clixon_util_xpath -p "/rpc-reply/datastores/datastore[name=\"$db\"]") resdb=${resdb0#"nodeset:0:"} diff --git a/test/test_restconf_op.sh b/test/test_restconf_op.sh index d4c0438c..cbf78ee2 100755 --- a/test/test_restconf_op.sh +++ b/test/test_restconf_op.sh @@ -172,8 +172,8 @@ expectpart "$(curl $CURLOPTS -X DELETE $RCPROTO://localhost/restconf/data)" 0 "H new "restconf GET null datastore" expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/example:cont1)" 0 "HTTP/$HVER 404" '{"ietf-restconf:errors":{"error":{"error-type":"application","error-tag":"invalid-value","error-severity":"error","error-message":"Instance does not exist"}}}' -new "restconf PUT initial datastore" -expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" -d '{"ietf-restconf:data":{"example:cont1":{"interface":{"name":"local0","type":"regular"}}}}' $RCPROTO://localhost/restconf/data)" 0 "HTTP/$HVER 204" # 201 Created (new resource) -> 204 No Content (existing modified) +new "restconf PUT initial datastore" +expectpart "$(curl $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" -d '{"ietf-restconf:data":{"example:cont1":{"interface":{"name":"local0","type":"regular"}}}}' $RCPROTO://localhost/restconf/data)" 0 "HTTP/$HVER 201" # 201 Created (new resource) -> 204 No Content (existing modified) new "restconf GET datastore" expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/example:cont1)" 0 "HTTP/$HVER 200" '{"example:cont1":{"interface":\[{"name":"local0","type":"regular"}\]}}' diff --git a/yang/clixon/clixon-config@2024-11-01.yang b/yang/clixon/clixon-config@2024-11-01.yang index f9098d1c..b0012fb2 100644 --- a/yang/clixon/clixon-config@2024-11-01.yang +++ b/yang/clixon/clixon-config@2024-11-01.yang @@ -60,7 +60,6 @@ module clixon-config { "Added options: CLICON_YANG_DOMAIN_DIR CLICON_YANG_USE_ORIGINAL - CLICON_XMLDB_SYSTEM_ONLY_CONFIG (tentative) Released in Clixon 7.2"; } revision 2024-04-01 { @@ -1206,8 +1205,13 @@ module clixon-config { default true; description "If set, some fields in the configuration tree are not stored to datastore. - Instead, the application must provide a mechanism to save the system-only-config + Instead, the application provides a mechanism to save the system-only-config in the system via commit/system-only-config callbacks. + Specifically, system-only data is read from the system except in the following case: + datastore is candidate, and either locked or modified + In that case, the system-only config is stored in the cache (not in file) and + not read from the system. + The system-only data is still not stored in the datastore however. See also extension system-only-config in clixon-lib.yang"; } leaf CLICON_XML_CHANGELOG {