Stricter incoming RPC sanity checking, error messages may have changed

This commit is contained in:
Olof hagsand 2020-04-07 20:54:24 +02:00
parent f9998c441c
commit ac5637a46a
30 changed files with 163 additions and 99 deletions

View file

@ -70,7 +70,7 @@ INCLUDES = -I. @INCLUDES@ -I$(top_srcdir)/lib/clixon -I$(top_srcdir)/include -I$
SRC = clixon_sig.c clixon_uid.c clixon_log.c clixon_err.c clixon_event.c \
clixon_string.c clixon_regex.c clixon_handle.c clixon_file.c \
clixon_xml.c clixon_xml_io.c clixon_xml_sort.c clixon_xml_map.c clixon_xml_vec.c \
clixon_json.c \
clixon_xml_bind.c clixon_json.c \
clixon_yang.c clixon_yang_type.c clixon_yang_module.c clixon_yang_parse_lib.c \
clixon_yang_cardinality.c clixon_xml_changelog.c clixon_xml_nsctx.c \
clixon_path.c clixon_validate.c \

View file

@ -362,6 +362,10 @@ text_modify(clicon_handle h,
}
}
if (x1bstr){
/* XXX: Do the type lookup here inline instead, there are more
* cases where we check for types, eg where we now call clixon_trim2
* AND in nacm path where we need proper namespace contexts.
*/
if (check_identityref(x1, x0, x0p, x1bstr, y0) < 0)
goto done;
if ((x0b = xml_body_get(x0)) != NULL){
@ -496,7 +500,7 @@ text_modify(clicon_handle h,
if (xml_copy(x1, x0) < 0)
goto done;
break;
}
} /* anyxml, anydata */
if (x0==NULL){
if (op==OP_MERGE && !permit && xnacm){
if ((ret = nacm_datanode_write(NULL, x1, NACM_CREATE, username, xnacm, cbret)) < 0)

View file

@ -45,6 +45,10 @@
* quoted-string = (DQUOTE string DQUOTE) / (SQUOTE string SQUOTE)
* positive-integer-value = (non-zero-digit DIGIT*)
* identifier = (ALPHA | "_")(ALPHA | DIGIT | "_" | "-" | ".")*
*
* RFC 8341: All the same rules as an instance-identifier apply, except that predicates
* for keys are optional. If a key predicate is missing, then the
* node-instance-identifier represents all possible server instances for that key.
*/
%start start

View file

@ -68,6 +68,7 @@
#include "clixon_yang_type.h"
#include "clixon_yang_module.h"
#include "clixon_xml_sort.h"
#include "clixon_xml_bind.h"
#include "clixon_xml_map.h"
#include "clixon_xml_nsctx.h" /* namespace context */
#include "clixon_netconf_lib.h"
@ -1249,6 +1250,12 @@ _json_parse(char *str,
break;
case YB_NONE:
break;
case YB_RPC:
if ((ret = xml_bind_yang_rpc(x, yspec, xerr)) < 0)
goto done;
if (ret == 0)
failed++;
break;
}
/* Now find leafs with identityrefs (+transitive) and translate
* prefixes in values to XML namespaces */

View file

@ -2,7 +2,8 @@
*
***** BEGIN LICENSE BLOCK *****
Copyright (C) 2009-2020 Olof Hagsand
Copyright (C) 2017-2019 Olof Hagsand
Copyright (C) 2020 Olof Hagsand and Rubicon Communications, LLC
This file is part of CLIXON.
@ -351,13 +352,13 @@ nacm_rule_datanode(cxobj *xt,
/* 6b) Either (1) the rule does not have a "rule-type" defined or
(2) the "rule-type" is "data-node" and the "path" matches the
requested data node, action node, or notification node. A
path is considered to match if the requested node is the node
specified by the path or is a descendant node of the path.*/
requested data node, action node, or notification node. */
if ((path = xml_find_body(xrule, "path")) == NULL){
if (xml_find_body(xrule, "rpc-name") ||xml_find_body(xrule, "notification-name"))
goto nomatch;
}
else
path = clixon_trim2(path, " \t\n");
access_operations = xml_find_body(xrule, "access-operations");
switch (access){
case NACM_READ:
@ -387,10 +388,17 @@ nacm_rule_datanode(cxobj *xt,
default:
break;
}
/* Here module is matched, now check for path if any NYI */
/* Here module is matched, now check for path if any NYI
* A path is considered to match if the requested node is the node
* specified by the path or is a descendant node of the path */
if (path){
#if 0
if ((ret = clixon_xml_find_instance_id(xt, yt, xvec, xlen, "%s", path)) == NULL)
goto nomatch;
#else
if ((xpath = xpath_first(xt, nsc, "%s", path)) == NULL)
goto nomatch;
#endif
/* The requested node xr is the node specified by the path or is a
* descendant node of the path:
* xmatch is one of xvec[] or an ancestor of the xvec[] nodes.

View file

@ -1321,9 +1321,9 @@ api_path_resolve(clixon_path *cplist,
* @retval -1 Error
* @retval 0 Fail
* @retval 1 OK
* @note: The spec says: prefixes depend on the XML context in which the value occurs. However,
* canonical prefixes/namespaces are used based on loaded yang modules.
* This means that prefix=NULL is not allowed.
* @note: The spec says: prefixes depend on the XML context in which the value occurs.
* However, canonical prefixes/namespaces are used based on loaded yang modules.
* This means that prefix=NULL is not allowed.
* Reasons for fail (retval = 0) are:
* - No prefix of identifier (keynames may omit prefix)
* - Prefix does not correspond to existing module.
@ -1564,7 +1564,8 @@ clixon_xml_find_api_path(cxobj *xt,
/*! Given (instance-id) path and XML tree, return matching xml node vector using stdarg
*
* Instance-identifier is a subset of XML XPaths and defined in Yang, used in NACM for example.
* Instance-identifier is a subset of XML XPaths and defined in Yang, used in NACM for
* example.
* @param[in] xt Top xml-tree where to search
* @param[in] yt Yang statement of top symbol (can be yang-spec if top-level)
* @param[out] xvec Vector of xml-trees. Vector must be free():d after use
@ -1588,6 +1589,7 @@ clixon_xml_find_api_path(cxobj *xt,
* }
* clixon_xvec_free(xvec);
* @endcode
* @note canonical namespace contexts are used, seexpath2canonical
* @see clixon_xml_find_api_path for RESTCONF api-paths
* @see RFC7950 Sec 9.13
*/

View file

@ -59,6 +59,7 @@
#include <netinet/in.h>
#include <sys/un.h>
#include <arpa/inet.h>
#include <assert.h>
/* cligen */
#include <cligen/cligen.h>
@ -170,15 +171,21 @@ clicon_msg_encode(uint32_t id,
* @param[in] yspec Yang specification, (can be NULL)
* @param[out] id Session id
* @param[out] xml XML parse tree
* @param[out] xerr Reason for failure (yang assignment not made) if retval =0
* @retval 1 Parse OK and all yang assignment made
* @retval 0 Parse OK but yang assigment not made (or only partial)
* @retval -1 Error with clicon_err called. Includes parse error
*/
int
clicon_msg_decode(struct clicon_msg *msg,
yang_stmt *yspec,
uint32_t *id,
cxobj **xml)
cxobj **xml,
cxobj **xerr)
{
int retval = -1;
char *xmlstr;
int retval = -1;
char *xmlstr;
int ret;
/* hdr */
if (id)
@ -186,11 +193,16 @@ clicon_msg_decode(struct clicon_msg *msg,
/* body */
xmlstr = msg->op_body;
clicon_debug(1, "%s %s", __FUNCTION__, xmlstr);
if (clixon_xml_parse_string(xmlstr, yspec?YB_MODULE:YB_NONE, yspec, xml, NULL) < 0)
if ((ret = clixon_xml_parse_string(xmlstr, yspec?YB_RPC:YB_NONE, yspec, xml, xerr)) < 0)
goto done;
retval = 0;
if (ret == 0)
goto fail;
retval = 1;
done:
return retval;
fail:
retval = 0;
goto done;
}
/*! Open local connection using unix domain sockets

View file

@ -76,7 +76,7 @@
#include "clixon_stream.h"
#include "clixon_err_string.h"
#include "clixon_xml_nsctx.h"
#include "clixon_xml_map.h"
#include "clixon_xml_bind.h"
#include "clixon_xml_sort.h"
#include "clixon_xml_io.h"
#include "clixon_netconf_lib.h"

View file

@ -2,7 +2,9 @@
*
***** BEGIN LICENSE BLOCK *****
Copyright (C) 2009-2019 Olof Hagsand and Benny Holmgren
Copyright (C) 2009-2016 Olof Hagsand and Benny Holmgren
Copyright (C) 2017-2019 Olof Hagsand
Copyright (C) 2020 Olof Hagsand and Rubicon Communications, LLC
This file is part of CLIXON.
@ -692,6 +694,29 @@ clixon_trim(char *str)
return s;
}
/*! Trim blanks from front and end of a string, return new string
* @param[in] str
* @param[in] trims Characters to trim: a vector of characters
* @retval s Pointer into existing str after trimming blanks
*/
char *
clixon_trim2(char *str,
char *trims)
{
char *s = str;
int i;
while (strlen(s) && index(trims, s[0])) /* trim from front */
s++;
for (i=strlen(s)-1; i>=0; i--){ /* trim from rear */
if (index(trims, s[i]))
s[i] = '\0';
else
break;
}
return s;
}
/*! check string equals (NULL is equal)
* @param[in] s1 String 1
* @param[in] s2 String 2

View file

@ -1090,14 +1090,10 @@ xml_yang_validate_all(clicon_handle h,
cprintf(cb, "Failed to find YANG spec of XML node: %s", xml_name(xt));
if ((xp = xml_parent(xt)) != NULL)
cprintf(cb, " with parent: %s", xml_name(xp));
if (namespace){
if (namespace)
cprintf(cb, " in namespace: %s", namespace);
if (netconf_unknown_element_xml(xret, "application", xml_name(xt), cbuf_get(cb)) < 0)
goto done;
}
else
if (netconf_unknown_element_xml(xret, "application", xml_name(xt), cbuf_get(cb)) < 0)
goto done;
if (netconf_unknown_element_xml(xret, "application", xml_name(xt), cbuf_get(cb)) < 0)
goto done;
goto fail;
}
if (yang_config(ys) != 0){

View file

@ -66,9 +66,9 @@
#include "clixon_log.h"
#include "clixon_yang.h"
#include "clixon_xml.h"
#include "clixon_options.h" /* xml_bind_yang */
#include "clixon_options.h"
#include "clixon_yang_module.h"
#include "clixon_xml_map.h" /* xml_bind_yang */
#include "clixon_xml_bind.h"
#include "clixon_xml_vec.h"
#include "clixon_xml_sort.h"
#include "clixon_xml_nsctx.h"
@ -455,7 +455,13 @@ _xml_parse(const char *str,
if (ret == 0)
failed++;
break;
}
case YB_RPC:
if ((ret = xml_bind_yang_rpc(x, yspec, xerr)) < 0)
goto done;
if (ret == 0)
failed++;
break;
} /* switch */
}
if (failed)
goto fail;
@ -601,7 +607,7 @@ clixon_xml_parse_file(int fd,
* @param[in] yb How to bind yang to XML top-level when parsing
* @param[in] yspec Yang specification, or NULL
* @param[in,out] xt Pointer to XML parse tree. If empty will be created.
* @param[out] xerr Reason for failure (yang assignment not made)
* @param[out] xerr Reason for failure (yang assignment not made) if retval = 0
* @retval 1 Parse OK and all yang assignment made
* @retval 0 Parse OK but yang assigment not made (or only partial)
* @retval -1 Error with clicon_err called. Includes parse error

View file

@ -1304,14 +1304,12 @@ xml_merge1(cxobj *x0, /* the target */
char *x1bstr; /* mod body string */
yang_stmt *yc; /* yang child */
cbuf *cbr = NULL; /* Reason buffer */
#ifdef XML_MERGE_TWO_ROUNDS
int i;
struct {
cxobj *w_x0c;
cxobj *w_x1c;
yang_stmt *w_yc;
} *second_wave = NULL;
#endif
assert(x1 && xml_type(x1) == CX_ELMNT);
assert(y0);
@ -1347,13 +1345,11 @@ xml_merge1(cxobj *x0, /* the target */
}
if (assign_namespaces(x1, x0, x0p) < 0)
goto done;
#ifdef XML_MERGE_TWO_ROUNDS
if ((second_wave = calloc(xml_child_nr(x1), sizeof(*second_wave))) == NULL){
clicon_err(OE_UNIX, errno, "calloc");
goto done;
}
i = 0;
#endif
/* Loop through children of the modification tree */
x1c = NULL;
while ((x1c = xml_child_each(x1, x1c, CX_ELMNT)) != NULL) {
@ -1377,7 +1373,6 @@ xml_merge1(cxobj *x0, /* the target */
x0c = NULL;
if (yc && match_base_child(x0, x1c, yc, &x0c) < 0)
goto done;
#ifdef XML_MERGE_TWO_ROUNDS
/* Save x0c, x1c, yc and merge in second wave, so that x1c entries "interfer"
* with itself, ie that later searches are among earlier objects already added
* to x0 */
@ -1385,14 +1380,7 @@ xml_merge1(cxobj *x0, /* the target */
second_wave[i].w_x1c = x1c;
second_wave[i].w_yc = yc;
i++;
#else
if (xml_merge1(x0c, yc, x0, x1c, reason) < 0)
goto done;
if (*reason != NULL)
goto ok;
#endif
} /* while */
#ifdef XML_MERGE_TWO_ROUNDS
/* Second run where actual merging is done
* Loop through children of the modification tree */
x1c = NULL;
@ -1408,7 +1396,6 @@ xml_merge1(cxobj *x0, /* the target */
goto ok;
i++;
}
#endif
if (xml_parent(x0) == NULL &&
xml_insert(x0p, x0, INS_LAST, NULL, NULL) < 0)
goto done;
@ -1416,10 +1403,8 @@ xml_merge1(cxobj *x0, /* the target */
ok:
retval = 0;
done:
#ifdef XML_MERGE_TWO_ROUNDS
if (second_wave)
free(second_wave);
#endif
if (cbr)
cbuf_free(cbr);
return retval;

View file

@ -660,7 +660,8 @@ cv_validate1(clicon_handle h,
if (strcmp(restype, "enumeration") == 0){
found = 0;
yi = NULL;
if (str != NULL)
if (str != NULL) {
str = clixon_trim2(str, " \t\n"); /* May be misplaced, strip earlier? */
while ((yi = yn_each(yrestype, yi)) != NULL){
if (yang_keyword_get(yi) != Y_ENUM)
continue;
@ -669,6 +670,7 @@ cv_validate1(clicon_handle h,
break;
}
}
}
if (!found){
if (reason)
*reason = cligen_reason("'%s' does not match enumeration", str);
@ -680,7 +682,7 @@ cv_validate1(clicon_handle h,
* of the names of the bits that are set. A zero-length string thus
* represents a value where no bits are set.
*/
str = clixon_trim2(str, " \t\n"); /* May be misplaced, strip earlier? */
nvec = 0;
if ((vec = clicon_strsep(str, " \t", &nvec)) == NULL)
goto done;