From bd290e4594dff66de9f9acb6c2c364356c8bddfd Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 24 Apr 2024 13:48:06 +0200 Subject: [PATCH] Fixed: Fail on return errors when reading from datastore --- CHANGELOG.md | 2 ++ apps/backend/backend_client.c | 21 ++++++++++++----- apps/backend/backend_commit.c | 4 ++-- apps/backend/backend_get.c | 31 +++++++++++++++++++++----- apps/backend/backend_plugin_restconf.c | 7 +++++- apps/backend/backend_startup.c | 6 ++++- example/main/example_backend.c | 7 +++++- lib/clixon/clixon_nacm.h | 2 +- lib/src/clixon_datastore_read.c | 5 ++++- lib/src/clixon_nacm.c | 22 +++++++++++++++--- 10 files changed, 87 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8b85100..f18794af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,8 @@ Expected: June 2024 ### Corrected Bugs +* Fixed: Fail on return errors when reading from datastore + * Can happen if running is not upgraded for example * Fixed: [Duplicate config files in configdir causes merge problems -> set ? = NULL](https://github.com/clicon/clixon/issues/510) ## 7.0.1 diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index 9ca339b1..dd3240d5 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -388,14 +388,17 @@ clixon_stats_datastore_get(clixon_handle h, uint64_t nr = 0; size_t sz = 0; cxobj *xn = NULL; + int ret; clixon_debug(CLIXON_DBG_BACKEND | CLIXON_DBG_DETAIL, "%s", dbname); /* This is the db cache */ if ((xt = xmldb_cache_get(h, dbname)) == NULL){ /* Trigger cache if no exist (trick to ensure cache is present) */ - if (xmldb_get(h, dbname, NULL, "/", &xn) < 0) + if ((ret = xmldb_get0(h, dbname, YB_MODULE, NULL, "/", 1, 0, &xn, NULL, NULL)) < 0) //goto done; goto ok; + if (ret == 0) + goto ok; xt = xmldb_cache_get(h, dbname); } if (xt != NULL){ @@ -1416,6 +1419,7 @@ from_client_stats(clixon_handle h, yang_stmt *yspec; yang_stmt *ymodext; cxobj *xt = NULL; + int ret; if ((str = xml_find_body(xe, "modules")) != NULL) modules = strcmp(str, "true") == 0; @@ -1470,8 +1474,12 @@ from_client_stats(clixon_handle h, cprintf(cbret, ""); /* Mountpoints */ if ((ymodext = yang_find(yspec, Y_MODULE, "ietf-yang-schema-mount")) != NULL){ - if (xmldb_get(h, "running", NULL, "/", &xt) < 0) + if ((ret = xmldb_get0(h, "running", YB_MODULE, NULL, "/", 1, 0, &xt, NULL, NULL)) < 0) goto done; + if (ret == 0){ + clixon_err(OE_DB, 0, "Error when reading from running, unknown error"); + goto done; + } if (xt && yang_schema_mount_statistics(h, xt, modules, cbret) < 0) goto done; } @@ -1793,12 +1801,15 @@ from_client_msg(clixon_handle h, xnacm = NULL; /* NACM intial pre- access control enforcements. Retval: - * 0: Use NACM validation and xnacm is set. - * 1: Permit, skip NACM + * 0: nacm declaration error + * 1: Use NACM validation and xnacm is set. + * 2: Permit, skip NACM * Therefore, xnacm=NULL means no NACM checks needed. */ - if ((ret = nacm_access_pre(h, ce->ce_username, username, &xnacm)) < 0) + if ((ret = nacm_access_pre(h, ce->ce_username, username, &xnacm, cbret)) < 0) goto done; + if (ret == 2) + goto reply; /* Cache XML NACM tree here. Use with caution, only valid on from_client_msg stack */ if (clicon_nacm_cache_set(h, xnacm) < 0) diff --git a/apps/backend/backend_commit.c b/apps/backend/backend_commit.c index c4b1805c..fadd7e1d 100644 --- a/apps/backend/backend_commit.c +++ b/apps/backend/backend_commit.c @@ -974,9 +974,9 @@ from_client_restart_one(clixon_handle h, if ((td = transaction_new()) == NULL) goto done; /* This is the state we are going to */ - if (xmldb_get0(h, "running", YB_MODULE, NULL, "/", 0, 0, &td->td_target, NULL, NULL) < 0) + if ((ret = xmldb_get0(h, "running", YB_MODULE, NULL, "/", 0, 0, &td->td_target, NULL, &xerr)) < 0) goto done; - if ((ret = xml_yang_validate_all_top(h, td->td_target, &xerr)) < 0) + if (ret == 1 && (ret = xml_yang_validate_all_top(h, td->td_target, &xerr)) < 0) goto done; if (ret == 0){ if (clixon_xml2cbuf(cbret, xerr, 0, 0, NULL, -1, 0) < 0) diff --git a/apps/backend/backend_get.c b/apps/backend/backend_get.c index 7049d18f..89a7323a 100644 --- a/apps/backend/backend_get.c +++ b/apps/backend/backend_get.c @@ -541,12 +541,12 @@ get_list_pagination(clixon_handle h, cbuf *cbmsg = NULL; /* For error msg */ cxobj *xret = NULL; char *xpath2; /* With optional pagination predicate */ - int ret; uint32_t iddb; /* DBs lock, if any */ int locked; cbuf *cberr = NULL; cxobj **xvec = NULL; size_t xlen; + int ret; #ifdef NOTYET cxobj *x; char *direction = NULL; @@ -651,7 +651,7 @@ get_list_pagination(clixon_handle h, /* Append predicate to original xpath and replace it */ xpath2 = cbuf_get(cbpath); /* specific xpath */ - if (xmldb_get0(h, db, YB_MODULE, nsc, xpath2?xpath2:"/", 1, wdef, &xret, NULL, NULL) < 0) { + if ((ret = xmldb_get0(h, db, YB_MODULE, nsc, xpath2?xpath2:"/", 1, wdef, &xret, NULL, &xerr)) < 0) { if ((cbmsg = cbuf_new()) == NULL){ clixon_err(OE_UNIX, errno, "cbuf_new"); goto done; @@ -661,6 +661,12 @@ get_list_pagination(clixon_handle h, goto done; goto ok; } + if (ret == 0){ + if (clixon_xml2cbuf(cbret, xerr, 0, 0, NULL, -1, 0) < 0) + goto done; + + goto ok; + } break; case CONTENT_NONCONFIG: /* state data only */ if ((xret = xml_new(DATASTORE_TOP_SYMBOL, NULL, CX_ELMNT)) == NULL)/* Only top tree */ @@ -885,7 +891,7 @@ get_common(clixon_handle h, switch (content){ case CONTENT_CONFIG: /* config data only */ /* specific xpath. with-default gets masked in get_nacm_and_reply */ - if (xmldb_get0(h, db, YB_MODULE, nsc, xpath?xpath:"/", 1, WITHDEFAULTS_REPORT_ALL, &xret, NULL, NULL) < 0) { + if ((ret = xmldb_get0(h, db, YB_MODULE, nsc, xpath?xpath:"/", 1, WITHDEFAULTS_REPORT_ALL, &xret, NULL, &xerr)) < 0) { if ((cbmsg = cbuf_new()) == NULL){ clixon_err(OE_UNIX, errno, "cbuf_new"); goto done; @@ -895,12 +901,17 @@ get_common(clixon_handle h, goto done; goto ok; } + if (ret == 0){ + if (clixon_xml2cbuf(cbret, xerr, 0, 0, NULL, -1, 0) < 0) + goto done; + goto ok; + } break; case CONTENT_ALL: /* both config and state */ case CONTENT_NONCONFIG: /* state data only */ if (clicon_option_bool(h, "CLICON_VALIDATE_STATE_XML")){ /* Whole config tree, for validate debug */ - if (xmldb_get0(h, "running", YB_MODULE, nsc, NULL, 1, WITHDEFAULTS_REPORT_ALL, &xret, NULL, NULL) < 0) { + if ((ret = xmldb_get0(h, "running", YB_MODULE, nsc, NULL, 1, WITHDEFAULTS_REPORT_ALL, &xret, NULL, &xerr)) < 0) { if ((cbmsg = cbuf_new()) == NULL){ clixon_err(OE_UNIX, errno, "cbuf_new"); goto done; @@ -910,10 +921,15 @@ get_common(clixon_handle h, goto done; goto ok; } + if (ret == 0){ + if (clixon_xml2cbuf(cbret, xerr, 0, 0, NULL, -1, 0) < 0) + goto done; + goto ok; + } } else if (content == CONTENT_ALL){ /* specific xpath */ - if (xmldb_get0(h, db, YB_MODULE, nsc, xpath?xpath:"/", 1, WITHDEFAULTS_REPORT_ALL, &xret, NULL, NULL) < 0) { + if ((ret = xmldb_get0(h, db, YB_MODULE, nsc, xpath?xpath:"/", 1, WITHDEFAULTS_REPORT_ALL, &xret, NULL, &xerr)) < 0) { if ((cbmsg = cbuf_new()) == NULL){ clixon_err(OE_UNIX, errno, "cbuf_new"); goto done; @@ -923,6 +939,11 @@ get_common(clixon_handle h, goto done; goto ok; } + if (ret == 0){ + if (clixon_xml2cbuf(cbret, xerr, 0, 0, NULL, -1, 0) < 0) + goto done; + goto ok; + } } /* CONTENT_NONCONFIG */ else if ((xret = xml_new(DATASTORE_TOP_SYMBOL, NULL, CX_ELMNT)) == NULL)/* Only top tree */ diff --git a/apps/backend/backend_plugin_restconf.c b/apps/backend/backend_plugin_restconf.c index b22a0225..cbdb75f7 100644 --- a/apps/backend/backend_plugin_restconf.c +++ b/apps/backend/backend_plugin_restconf.c @@ -202,6 +202,7 @@ restconf_rpc_wrapper(clixon_handle h, { int retval = -1; cxobj *xt = NULL; + int ret; clixon_debug(CLIXON_DBG_BACKEND, ""); switch (*operation){ @@ -211,8 +212,12 @@ restconf_rpc_wrapper(clixon_handle h, case PROC_OP_START: /* RPC op is start & enable is true, then start the service, & enable is false, error or ignore it */ - if (xmldb_get(h, "running", NULL, "/restconf", &xt) < 0) + if ((ret = xmldb_get0(h, "running", YB_MODULE, NULL, "/restconf", 1, 0, &xt, NULL, NULL)) < 0) goto done; + if (ret == 0){ + clixon_err(OE_DB, 0, "Error when reading from running, unknown error"); + goto done; + } if (xt != NULL && xpath_first(xt, NULL, "/restconf[enable='false']") != NULL) { *operation = PROC_OP_NONE; diff --git a/apps/backend/backend_startup.c b/apps/backend/backend_startup.c index b1ead1ba..5f0d19f2 100644 --- a/apps/backend/backend_startup.c +++ b/apps/backend/backend_startup.c @@ -309,8 +309,12 @@ startup_extraxml(clixon_handle h, * It should be empty if extra-xml is null and reset plugins did nothing * then skip validation. */ - if (xmldb_get(h, tmp_db, NULL, NULL, &xt0) < 0) + if ((ret = xmldb_get0(h, tmp_db, YB_MODULE, NULL, NULL, 1, 0, &xt0, NULL, NULL)) < 0) goto done; + if (ret == 0){ + clixon_err(OE_DB, 0, "Error when reading from %s, unknown error", tmp_db); + goto done; + } if ((ret = xmldb_empty_get(h, tmp_db)) < 0) goto done; if (ret == 1) diff --git a/example/main/example_backend.c b/example/main/example_backend.c index f0327960..1b4ad215 100644 --- a/example/main/example_backend.c +++ b/example/main/example_backend.c @@ -467,6 +467,7 @@ example_statedata(clixon_handle h, char *name; cvec *nsc1 = NULL; yang_stmt *yspec = NULL; + int ret; if (!_state) goto ok; @@ -481,8 +482,12 @@ example_statedata(clixon_handle h, * Get config according to xpath */ if ((nsc1 = xml_nsctx_init(NULL, "urn:ietf:params:xml:ns:yang:ietf-interfaces")) == NULL) goto done; - if (xmldb_get0(h, "running", YB_MODULE, nsc1, "/interfaces/interface/name", 1, 0, &xt, NULL, NULL) < 0) + if ((ret = xmldb_get0(h, "running", YB_MODULE, nsc1, "/interfaces/interface/name", 1, 0, &xt, NULL, NULL)) < 0) goto done; + if (ret == 0){ + clixon_err(OE_DB, 0, "Error when reading from running, unknown error"); + goto done; + } if (xpath_vec(xt, nsc1, "/interfaces/interface/name", &xvec, &xlen) < 0) goto done; if (xlen){ diff --git a/lib/clixon/clixon_nacm.h b/lib/clixon/clixon_nacm.h index 651a0221..188fd68b 100644 --- a/lib/clixon/clixon_nacm.h +++ b/lib/clixon/clixon_nacm.h @@ -62,7 +62,7 @@ int nacm_datanode_read(clixon_handle h, cxobj *xt, cxobj **xvec, size_t xlen, ch int nacm_datanode_write(clixon_handle h, cxobj *xr, cxobj *xt, enum nacm_access access, char *username, cxobj *xnacm, cbuf *cbret); -int nacm_access_pre(clixon_handle h, char *peername, char *username, cxobj **xnacmp); +int nacm_access_pre(clixon_handle h, char *peername, char *username, cxobj **xnacmp, cbuf *cbret); int verify_nacm_user(clixon_handle h, enum nacm_credentials_t cred, char *peername, char *nacmname, char *rpcname, cbuf *cbret); #endif /* _CLIXON_NACM_H */ diff --git a/lib/src/clixon_datastore_read.c b/lib/src/clixon_datastore_read.c index a15c0807..078d22ff 100644 --- a/lib/src/clixon_datastore_read.c +++ b/lib/src/clixon_datastore_read.c @@ -817,11 +817,14 @@ xmldb_get_cache(clixon_handle h, * @retval -1 Error * @note Use of 1 for OK * @code - * if (xmldb_get(xh, "running", NULL, "/interfaces/interface[name="eth"]", &xt) < 0) + * if ((ret = xmldb_get(xh, "running", NULL, "/interfaces/interface[name="eth"]", &xt)) < 0) + * err; + * if (ret == 0) * err; * xml_free(xt); * @endcode * @see xmldb_get0 Underlying more capable API for enabling zero-copy + * XXX: OBSOLETE: use xmldb_get0 directly */ int xmldb_get(clixon_handle h, diff --git a/lib/src/clixon_nacm.c b/lib/src/clixon_nacm.c index baa8efdb..1dbddccf 100644 --- a/lib/src/clixon_nacm.c +++ b/lib/src/clixon_nacm.c @@ -72,6 +72,7 @@ #include "clixon_datastore.h" #include "clixon_xml_nsctx.h" #include "clixon_xml_map.h" +#include "clixon_xml_io.h" #include "clixon_path.h" #include "clixon_xml_vec.h" #include "clixon_nacm.h" @@ -1196,7 +1197,9 @@ nacm_access_check(clixon_handle h, * @param[in] h Clixon handle * @param[in] peername Peer username if any * @param[in] username User name of requestor - * @param[out] xncam NACM XML tree, set if retval=0. Free after use + * @param[out] xnacm NACM XML tree, set if retval=0. Free after use + * @param[out] cbret Error if ret == 2 + * @retval 2 Failed on reading NACM from running (internal), cbret has error * @retval 1 OK permitted. You do not need to do next NACM step. * @retval 0 OK but not validated. Need to do NACM step using xnacm * @retval -1 Error @@ -1215,7 +1218,8 @@ int nacm_access_pre(clixon_handle h, char *peername, char *username, - cxobj **xnacmp) + cxobj **xnacmp, + cbuf *cbret) { int retval = -1; char *mode; @@ -1223,6 +1227,8 @@ nacm_access_pre(clixon_handle h, cxobj *xnacm0 = NULL; cxobj *xnacm = NULL; cvec *nsc = NULL; + cxobj *xerr = NULL; + int ret; /* Check clixon option: disabled, external tree or internal */ mode = clicon_option_str(h, "CLICON_NACM_MODE"); @@ -1236,8 +1242,13 @@ nacm_access_pre(clixon_handle h, goto done; } else if (strcmp(mode, "internal")==0){ - if (xmldb_get0(h, "running", YB_MODULE, nsc, "nacm", 1, 0, &xnacm0, NULL, NULL) < 0) + if ((ret = xmldb_get0(h, "running", YB_MODULE, nsc, "nacm", 1, 0, &xnacm0, NULL, &xerr)) < 0) goto done; + if (ret == 0){ + if (clixon_xml2cbuf(cbret, xerr, 0, 0, NULL, -1, 0) < 0) + goto done; + goto fail; + } } else{ clixon_err(OE_XML, 0, "Invalid NACM mode: %s", mode); @@ -1268,10 +1279,15 @@ nacm_access_pre(clixon_handle h, xml_free(xnacm0); else if (xnacm) xml_free(xnacm); + else if (xerr) + xml_free(xerr); return retval; permit: retval = 1; goto done; + fail: + retval = 2; + goto done; } /*! Verify nacm user with peer uid credentials