diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7fd27e05..788559b2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -49,6 +49,9 @@ Thanks netgate for providing the dispatcher code!
Users may have to change how they access the system
+* RPC replies now verified with YANG
+ * Stricter checking of outgoing RPC replies from server
+ * See [RPC output not verified by yang](https://github.com/clicon/clixon/issues/283)
* XML to JSON CDATA translation is NOT stripped
* Example, assume XML: ` x & x < y ]]>`
* Previous bevavior:
@@ -100,6 +103,7 @@ Developers may need to change their code
* Any changes to context are logged at loglevel WARNING
* [OpenConfig path compression](https://github.com/clicon/clixon/pull/276)
* C API: Added set/get pointer API to clixon_data:
+ * Changed signature of `rpc_callback_call()`
* Added json/cli support for cli save/load
* clicon_ptr_get(), clicon_ptr_set(),
* Restconf YANG PATCH according to RFC 8072
@@ -110,6 +114,7 @@ Developers may need to change their code
### Corrected Bugs
+* [RPC output not verified by yang](https://github.com/clicon/clixon/issues/283)
* [Statements given in "load set" are order dependent](https://github.com/clicon/clixon/issues/287)
* Modify ordering of XML encoding to put sub-elements with YANG WHEN statements last
* [RPC get-conf method returned some content not specified by select filter](https://github.com/clicon/clixon/issues/281)
diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c
index cad66831..20700d49 100644
--- a/apps/backend/backend_client.c
+++ b/apps/backend/backend_client.c
@@ -217,14 +217,15 @@ clixon_stats_get_db(clicon_handle h,
xt = xmldb_cache_get(h, dbname);
}
if (xt == NULL){
- cprintf(cb, "%s00", dbname);
+ cprintf(cb, "%s00",
+ CLIXON_LIB_NS, dbname);
}
else{
if (xml_stats(xt, &nr, &sz) < 0)
goto done;
- cprintf(cb, "%s%" PRIu64 ""
+ cprintf(cb, "%s%" PRIu64 ""
"%zu",
- dbname, nr, sz);
+ CLIXON_LIB_NS, dbname, nr, sz);
}
retval = 0;
done:
@@ -1001,7 +1002,7 @@ from_client_stats(clicon_handle h,
cprintf(cbret, "", NETCONF_BASE_NAMESPACE);
nr=0;
xml_stats_global(&nr);
- cprintf(cbret, "%" PRIu64 "", nr);
+ cprintf(cbret, "%" PRIu64 "", CLIXON_LIB_NS, nr);
if (clixon_stats_get_db(h, "running", cbret) < 0)
goto done;
if (clixon_stats_get_db(h, "candidate", cbret) < 0)
@@ -1136,6 +1137,7 @@ from_client_hello(clicon_handle h,
return retval;
}
+
/*! An internal clicon message has arrived from a client. Receive and dispatch.
* @param[in] h Clicon handle
* @param[in] s Socket where message arrived. read from this.
@@ -1168,6 +1170,7 @@ from_client_msg(clicon_handle h,
char *rpcname;
char *rpcprefix;
char *namespace = NULL;
+ int nr = 0;
clicon_debug(1, "%s", __FUNCTION__);
yspec = clicon_dbspec_yang(h);
@@ -1295,13 +1298,15 @@ from_client_msg(clicon_handle h,
goto reply;
}
clicon_err_reset();
- if ((ret = rpc_callback_call(h, xe, cbret, ce)) < 0){
+ if ((ret = rpc_callback_call(h, xe, ce, &nr, cbret)) < 0){
if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0)
goto done;
clicon_log(LOG_NOTICE, "%s Error in rpc_callback_call:%s", __FUNCTION__, xml_name(xe));
goto reply; /* Dont quit here on user callbacks */
}
- if (ret == 0){ /* not handled by callback */
+ if (ret == 0)
+ goto reply;
+ if (nr == 0){ /* not handled by callback */
if (netconf_operation_not_supported(cbret, "application", "RPC operation not supported")< 0)
goto done;
goto reply;
diff --git a/apps/netconf/netconf_rpc.c b/apps/netconf/netconf_rpc.c
index 8ad952cf..30cd7073 100644
--- a/apps/netconf/netconf_rpc.c
+++ b/apps/netconf/netconf_rpc.c
@@ -588,6 +588,7 @@ netconf_application_rpc(clicon_handle h,
cbuf *cb = NULL;
cbuf *cbret = NULL;
int ret;
+ int nr = 0;
/* First check system / netconf RPC:s */
if ((cb = cbuf_new()) == NULL){
@@ -623,9 +624,13 @@ netconf_application_rpc(clicon_handle h,
if (yrpc != NULL){
/* No need to check xn arguments with input statement since already bound and validated. */
/* Look for local (client-side) netconf plugins. */
- if ((ret = rpc_callback_call(h, xn, cbret, NULL)) < 0)
+ if ((ret = rpc_callback_call(h, xn, NULL, &nr, cbret)) < 0)
goto done;
- if (ret > 0){ /* Handled locally */
+ if (ret == 0){
+ if (clixon_xml_parse_string(cbuf_get(cbret), YB_NONE, NULL, xret, NULL) < 0)
+ goto done;
+ }
+ else if (nr > 0){ /* Handled locally */
if (clixon_xml_parse_string(cbuf_get(cbret), YB_NONE, NULL, xret, NULL) < 0)
goto done;
}
diff --git a/apps/restconf/restconf_methods_post.c b/apps/restconf/restconf_methods_post.c
index 23a627d5..31976596 100644
--- a/apps/restconf/restconf_methods_post.c
+++ b/apps/restconf/restconf_methods_post.c
@@ -712,6 +712,7 @@ api_operations_post(clicon_handle h,
char *id = NULL;
yang_stmt *ys = NULL;
char *namespace = NULL;
+ int nr = 0;
clicon_debug(1, "%s json:\"%s\" path:\"%s\"", __FUNCTION__, data, api_path);
/* 1. Initialize */
@@ -826,9 +827,16 @@ api_operations_post(clicon_handle h,
/* Look for local (client-side) restconf plugins.
* -1:Error, 0:OK local, 1:OK backend
*/
- if ((ret = rpc_callback_call(h, xbot, cbret, req)) < 0)
+ if ((ret = rpc_callback_call(h, xbot, req, &nr, cbret)) < 0)
goto done;
- if (ret > 0){ /* Handled locally */
+ if (ret == 0){
+ if (clixon_xml_parse_string(cbuf_get(cbret), YB_NONE, NULL, &xe, NULL) < 0)
+ goto done;
+ if (api_return_err(h, req, xe, pretty, media_out, 0) < 0)
+ goto done;
+ goto ok;
+ }
+ else if (nr > 0){ /* Handled locally */
if (clixon_xml_parse_string(cbuf_get(cbret), YB_NONE, NULL, &xret, NULL) < 0)
goto done;
/* Local error: return it and quit */
diff --git a/lib/clixon/clixon_plugin.h b/lib/clixon/clixon_plugin.h
index c7294881..feddc4a6 100644
--- a/lib/clixon/clixon_plugin.h
+++ b/lib/clixon/clixon_plugin.h
@@ -395,8 +395,7 @@ int clixon_plugin_datastore_upgrade_all(clicon_handle h, const char *db, cxobj *
/* rpc callback API */
int rpc_callback_register(clicon_handle h, clicon_rpc_cb cb, void *arg, const char *ns, const char *name);
-int rpc_callback_call(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg);
-
+int rpc_callback_call(clicon_handle h, cxobj *xe, void *arg, int *nrp, cbuf *cbret);
/* upgrade callback API */
int upgrade_callback_reg_fn(clicon_handle h, clicon_upgrade_cb cb, const char *strfn, const char *ns, void *arg);
int upgrade_callback_call(clicon_handle h, cxobj *xt, char *ns, uint16_t op, uint32_t from, uint32_t to, cbuf *cbret);
diff --git a/lib/clixon/clixon_validate.h b/lib/clixon/clixon_validate.h
index 45720d39..7af5c338 100644
--- a/lib/clixon/clixon_validate.h
+++ b/lib/clixon/clixon_validate.h
@@ -44,9 +44,11 @@
* Prototypes
*/
int xml_yang_validate_rpc(clicon_handle h, cxobj *xrpc, cxobj **xret);
+int xml_yang_validate_rpc_reply(clicon_handle h, cxobj *xrpc, cxobj **xret);
int xml_yang_validate_add(clicon_handle h, cxobj *xt, cxobj **xret);
int xml_yang_validate_list_key_only(cxobj *xt, cxobj **xret);
int xml_yang_validate_all(clicon_handle h, cxobj *xt, cxobj **xret);
int xml_yang_validate_all_top(clicon_handle h, cxobj *xt, cxobj **xret);
+int rpc_reply_check(clicon_handle h, char *rpcname, cbuf *cbret);
#endif /* _CLIXON_VALIDATE_H_ */
diff --git a/lib/clixon/clixon_xml.h b/lib/clixon/clixon_xml.h
index 03766c92..f77a5690 100644
--- a/lib/clixon/clixon_xml.h
+++ b/lib/clixon/clixon_xml.h
@@ -147,7 +147,7 @@ enum yang_bind{
* Ie, xml looks like: ... where "x" is a top-level symbol in a module
*/
YB_PARENT, /* Assume yang binding of existing parent and match its children by name */
- YB_RPC, /* Assume top-level xml is an netconf RPC message (or hello) */
+ YB_RPC, /* Assume top-level xml is an incoming netconf RPC message (or hello) */
};
typedef enum yang_bind yang_bind;
diff --git a/lib/src/clixon_plugin.c b/lib/src/clixon_plugin.c
index 2bebbcf3..a14cdf03 100644
--- a/lib/src/clixon_plugin.c
+++ b/lib/src/clixon_plugin.c
@@ -66,6 +66,7 @@
#include "clixon_xml.h"
#include "clixon_xml_nsctx.h"
#include "clixon_yang_module.h"
+#include "clixon_validate.h"
#include "clixon_plugin.h"
/*
@@ -1093,13 +1094,14 @@ rpc_callback_delete_all(clicon_handle h)
/*! Search RPC callbacks and invoke if XML match with tag
*
- * @param[in] h clicon handle
- * @param[in] xn Sub-tree (under xorig) at child of rpc: .
- * @param[out] cbret Return XML (as string in CLIgen buffer), error or OK
- * @param[in] arg Domain-speific arg (eg client_entry)
+ * @param[in] h clicon handle
+ * @param[in] xn Sub-tree (under xorig) at child of rpc: .
+ * @param[in] arg Domain-speific arg (eg client_entry)
+ * @param[out] nr Number of callbacks handled: 0, 1, n (retval = 1)
+ * @param[out] cbret Return XML (as string in CLIgen buffer), error or OK
* @retval -1 Error
- * @retval 0 OK, not found handler.
- * @retval n OK, handler called
+ * @retval 0 Failed, error return in cbret
+ * @retval 1 OK, see nr
* @see rpc_callback_register which register a callback function
* @note that several callbacks can be registered. They need to cooperate on
* return values, ie if one writes cbret, the other needs to handle that by
@@ -1108,8 +1110,9 @@ rpc_callback_delete_all(clicon_handle h)
int
rpc_callback_call(clicon_handle h,
cxobj *xe,
- cbuf *cbret,
- void *arg)
+ void *arg,
+ int *nrp,
+ cbuf *cbret)
{
int retval = -1;
rpc_callback_t *rc;
@@ -1119,6 +1122,7 @@ rpc_callback_call(clicon_handle h,
int nr = 0; /* How many callbacks */
plugin_module_struct *ms = plugin_module_struct_get(h);
plugin_context_t *pc = NULL;
+ int ret;
if (ms == NULL){
clicon_err(OE_PLUGIN, EINVAL, "plugin module not initialized");
@@ -1148,12 +1152,22 @@ rpc_callback_call(clicon_handle h,
}
rc = NEXTQ(rpc_callback_t *, rc);
} while (rc != ms->ms_rpc_callbacks);
- retval = nr; /* 0: none found, >0 nr of handlers called */
+ if (nr){
+ if ((ret = rpc_reply_check(h, name, cbret)) < 0)
+ goto done;
+ if (ret == 0)
+ goto fail;
+ }
+ *nrp = nr;
+ retval = 1; /* 0: none found, >0 nr of handlers called */
done:
clicon_debug(1, "%s retval:%d", __FUNCTION__, retval);
if (pc)
free(pc);
return retval;
+ fail:
+ retval = 0;
+ goto done;
}
/*--------------------------------------------------------------------
diff --git a/lib/src/clixon_validate.c b/lib/src/clixon_validate.c
index 743b213e..b8a988fd 100644
--- a/lib/src/clixon_validate.c
+++ b/lib/src/clixon_validate.c
@@ -69,6 +69,7 @@
#include "clixon_log.h"
#include "clixon_yang.h"
#include "clixon_xml.h"
+#include "clixon_data.h"
#include "clixon_netconf_lib.h"
#include "clixon_options.h"
#include "clixon_xml_nsctx.h"
@@ -78,6 +79,7 @@
#include "clixon_yang_module.h"
#include "clixon_yang_type.h"
#include "clixon_xml_map.h"
+#include "clixon_xml_bind.h"
#include "clixon_validate.h"
/*! Validate xml node of type leafref, ensure the value is one of that path's reference
@@ -419,6 +421,67 @@ xml_yang_validate_rpc(clicon_handle h,
goto done;
}
+int
+xml_yang_validate_rpc_reply(clicon_handle h,
+ cxobj *xrpc,
+ cxobj **xret)
+{
+ int retval = -1;
+ yang_stmt *yn=NULL; /* rpc name */
+ cxobj *xn; /* rpc name */
+ char *rpcprefix;
+ char *namespace = NULL;
+ int ret;
+
+ if (strcmp(xml_name(xrpc), "rpc-reply")){
+ clicon_err(OE_XML, EINVAL, "Expected RPC");
+ goto done;
+ }
+ rpcprefix = xml_prefix(xrpc);
+ if (xml2ns(xrpc, rpcprefix, &namespace) < 0)
+ goto done;
+ /* Only accept resolved NETCONF base namespace */
+ if (namespace == NULL || strcmp(namespace, NETCONF_BASE_NAMESPACE) != 0){
+ if (xret && netconf_unknown_namespace_xml(xret, "protocol",
+ rpcprefix?rpcprefix:"null",
+ "No appropriate namespace associated with prefix")< 0)
+ goto done;
+ goto fail;
+ }
+ xn = NULL;
+ /* xn is name of rpc, ie */
+ while ((xn = xml_child_each(xrpc, xn, CX_ELMNT)) != NULL) {
+ /* OK and rpc-error are not explicit in ietf-netconf.yang. Why are they hardcoded ? */
+ if (strcmp(xml_name(xn), "ok") == 0 || strcmp(xml_name(xn), "rpc-error") == 0){
+ continue;
+ }
+ if ((yn = xml_spec(xn)) == NULL){
+ if (xret && netconf_unknown_element_xml(xret, "application", xml_name(xn), NULL) < 0)
+ goto done;
+ goto fail;
+ }
+ if ((ret = xml_yang_validate_all(h, xn, xret)) < 0)
+ goto done; /* error or validation fail */
+ if (ret == 0)
+ goto fail;
+ if ((ret = xml_yang_validate_add(h, xn, xret)) < 0)
+ goto done; /* error or validation fail */
+ if (ret == 0)
+ goto fail;
+ if (xml_default_recurse(xn, 0) < 0)
+ goto done;
+ }
+ // ok: /* pass validation */
+ retval = 1;
+ done:
+ return retval;
+ fail:
+ if (xret && *xret && clixon_xml_attr_copy(xrpc, *xret, "message-id") < 0)
+ goto done;
+ retval = 0;
+ goto done;
+}
+
/*! Check if an xml node is a part of a choice and have >1 siblings
* @param[in] xt XML node to be validated
* @param[in] yt xt:s yang statement
@@ -1405,3 +1468,64 @@ xml_yang_validate_all_top(clicon_handle h,
return ret;
return 1;
}
+
+/*! Check validity of outgoing RPC
+ *
+ * Rewrite return message if errors
+ * @param[in,out] cbret
+ * @note Parses cbret which seems one time too many
+ */
+int
+rpc_reply_check(clicon_handle h,
+ char *rpcname,
+ cbuf *cbret)
+{
+ int retval = -1;
+ cxobj *x = NULL;
+ cxobj *xret = NULL;
+ int ret;
+ yang_stmt *yspec;
+
+ if ((yspec = clicon_dbspec_yang(h)) == NULL){
+ clicon_err(OE_YANG, ENOENT, "No yang spec9");
+ goto done;
+ }
+ /* Validate outgoing RPC */
+ if ((ret = clixon_xml_parse_string(cbuf_get(cbret), YB_NONE, NULL, &x, NULL)) < 0)
+ goto done;
+ if (xml_child_nr(x) == 0){
+ cbuf_reset(cbret);
+ if (netconf_operation_failed(cbret, "application",
+ "Internal error: Outgoing reply is empty")< 0)
+ goto done;
+ goto fail;
+ }
+ if (xml_rootchild(x, 0, &x) < 0)
+ goto done;
+ if ((ret = xml_bind_yang_rpc_reply(x, rpcname, yspec, &xret)) < 0)
+ goto done;
+ if (ret == 0){
+ cbuf_reset(cbret);
+ if (clicon_xml2cbuf(cbret, xret, 0, 0, -1) < 0)
+ goto done;
+ goto fail;
+ }
+ if ((ret = xml_yang_validate_rpc_reply(h, x, &xret)) < 0)
+ goto done;
+ if (ret == 0){
+ cbuf_reset(cbret);
+ if (clicon_xml2cbuf(cbret, xret, 0, 0, -1) < 0)
+ goto done;
+ goto fail;
+ }
+ retval = 1;
+ done:
+ if (x)
+ xml_free(x);
+ if (xret)
+ xml_free(xret);
+ return retval;
+ fail:
+ retval = 0;
+ goto done;
+}
diff --git a/lib/src/clixon_xml_bind.c b/lib/src/clixon_xml_bind.c
index c625e93d..5245751b 100644
--- a/lib/src/clixon_xml_bind.c
+++ b/lib/src/clixon_xml_bind.c
@@ -521,11 +521,11 @@ xml_bind_yang0(cxobj *xt,
* @retval -1 Error
* The
* @code
- * if (xml_bind_yang_rpc(x, NULL) < 0)
+ * if ((ret = xml_bind_yang_rpc(x, NULL, &xerr)) < 0)
* err;
* @endcode
* @see xml_bind_yang For other generic cases
- * @see xml_bind_yang_rpc_reply
+ * @see xml_bind_yang_rpc_reply
*/
int
xml_bind_yang_rpc(cxobj *xrpc,
@@ -666,7 +666,7 @@ xml_bind_yang_rpc(cxobj *xrpc,
* @retval -1 Error
*
* @code
- * if (xml_bind_yang_rpc_reply(x, "get-config", yspec, name) < 0)
+ * if ((ret = xml_bind_yang_rpc_reply(x, "get-config", yspec, &xerr)) < 0)
* err;
* @endcode
* @see xml_bind_yang For other generic cases
@@ -686,6 +686,7 @@ xml_bind_yang_rpc_reply(cxobj *xrpc,
cxobj *xerr1 = NULL;
char *opname;
cbuf *cberr = NULL;
+ cxobj *xc;
opname = xml_name(xrpc);
if (strcmp(opname, "rpc-reply")){
@@ -716,6 +717,13 @@ xml_bind_yang_rpc_reply(cxobj *xrpc,
}
if (yo != NULL){
xml_spec_set(xrpc, yo);
+ /* Special case for ok and rpc-error */
+ if ((xc = xml_child_i_type(xrpc, 0, CX_ELMNT)) != NULL &&
+ (strcmp(xml_name(xc),"rpc-error") == 0
+ || strcmp(xml_name(xc),"ok") == 0
+ )){
+ goto ok;
+ }
/* Use a temporary xml error tree since it is stringified in the original error on error */
if ((ret = xml_bind_yang(xrpc, YB_PARENT, NULL, &xerr1)) < 0)
goto done;
@@ -732,6 +740,7 @@ xml_bind_yang_rpc_reply(cxobj *xrpc,
goto fail;
}
}
+ ok:
retval = 1;
done:
if (cberr)
diff --git a/test/test_c++.sh b/test/test_c++.sh
index c9be6f2a..ad4725ac 100755
--- a/test/test_c++.sh
+++ b/test/test_c++.sh
@@ -101,7 +101,7 @@ int example_rpc(clicon_handle h,
clicon_err(OE_XML, ENOENT, "No namespace given in rpc %s", xml_name(xe));
goto done;
}
- cprintf(cbret, "");
+ cprintf(cbret, "", NETCONF_BASE_NAMESPACE);
if (!xml_child_nr_type(xe, CX_ELMNT))
cprintf(cbret, "");
else