diff --git a/CHANGELOG.md b/CHANGELOG.md index da8a4225..748923d8 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/apps/backend/backend_main.c b/apps/backend/backend_main.c index 79b86ee2..aed5a80d 100644 --- a/apps/backend/backend_main.c +++ b/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 (xmldb_exists(h, "startup") != 1) - if (xmldb_create(h, "startup") < 0) + 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 (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 */ diff --git a/lib/src/clixon_datastore.c b/lib/src/clixon_datastore.c index bc5f09a4..7540dbf6 100644 --- a/lib/src/clixon_datastore.c +++ b/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 - retval = 1; + + 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,24 +421,16 @@ 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. - // It was changed back for confirmed-commit, and test_confirmed_commit.sh drops privileges. - // 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; - } + if (truncate(filename, 0) < 0){ + clicon_err(OE_DB, errno, "truncate %s", filename); + goto done; + } retval = 0; done: if (filename) @@ -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) { - char *old; - char *fname = NULL; - int retval = -1; + int retval = -1; + char *old; + char *fname = NULL; + 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; - - newdb = newdb == NULL ? old : newdb; - 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; - }; - - int actual = 0; - if ((actual = snprintf(fname, size, "%s%s", newdb, suffix)) < size) { - clicon_err(OE_UNIX, 0, "snprintf wrote fewer bytes (%d) than requested (%zu)", actual, size); - goto done; - }; - + if ((cb = cbuf_new()) == NULL){ + clicon_err(OE_XML, errno, "cbuf_new"); + goto done; + } + cprintf(cb, "%s", newdb == NULL ? old : newdb); + if (suffix) + cprintf(cb, "%s", suffix); + fname = cbuf_get(cb); if ((rename(old, fname)) < 0) { clicon_err(OE_UNIX, errno, "rename: %s", strerror(errno)); goto done; }; - - retval = 0; - - done: + done: + if (cb) + cbuf_free(cb); if (old) free(old); - - if (fname) - free(fname); - return retval; } diff --git a/test/test_confirmed_commit.sh b/test/test_confirmed_commit.sh index a805ce27..4abcb8be 100755 --- a/test/test_confirmed_commit.sh +++ b/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="