* Fixed: SEGV in clixon_netconf_lib functions from internal errors including validation.

* Check xerr argument both before and after call on netconf lib functions
This commit is contained in:
Olof hagsand 2021-07-27 09:39:44 +02:00
parent 600f29a117
commit dee081646c
9 changed files with 103 additions and 39 deletions

View file

@ -48,6 +48,8 @@ Developers may need to change their code
### Corrected Bugs
* Fixed: SEGV in clixon_netconf_lib functions from internal errors including validation.
* Check xerr argument both before and after call on netconf lib functions
* Fixed: RFC 8040 yang-data extension allows non-key lists
* Added YANG_FLAG_NOKEY as exception to mandatory key lists
* Fixed: mandatory leaf in a uses statement caused abort

View file

@ -238,7 +238,7 @@ client_get_streams(clicon_handle h,
cprintf(cb,"</%s>", top);
if (clixon_xml_parse_string(cbuf_get(cb), YB_MODULE, yspec, &x, NULL) < 0){
if (netconf_operation_failed_xml(xret, "protocol", clicon_err_reason)< 0)
if (xret && netconf_operation_failed_xml(xret, "protocol", clicon_err_reason)< 0)
goto done;
goto fail;
}

View file

@ -132,7 +132,7 @@ generic_validate(clicon_handle h,
cprintf(cb, "Mandatory variable of %s in module %s",
xml_parent(x1)?xml_name(xml_parent(x1)):"",
yang_argument_get(ys_module(ys)));
if (netconf_missing_element_xml(xret, "protocol", xml_name(x1), cbuf_get(cb)) < 0)
if (xret && netconf_missing_element_xml(xret, "protocol", xml_name(x1), cbuf_get(cb)) < 0)
goto done;
goto fail;
}
@ -480,9 +480,9 @@ startup_commit(clicon_handle h,
* and call application callback validations.
* @param[in] h Clicon handle
* @param[in] candidate The candidate database. The wanted backend state
* @param[out] xret Error XML tree. Free with xml_free after use
* @param[out] xret Error XML tree, if retval is 0. Free with xml_free after use
* @retval -1 Error - or validation failed (but cbret not set)
* @retval 0 Validation failed (with cbret set)
* @retval 0 Validation failed (with xret set)
* @retval 1 Validation OK
* @note Need to differentiate between error and validation fail
* (only done for generic_validate)
@ -505,16 +505,19 @@ validate_common(clicon_handle h,
goto done;
}
/* This is the state we are going to */
if (xmldb_get0(h, db, YB_MODULE, NULL, "/", 0, &td->td_target, NULL, NULL) < 0)
if ((ret = xmldb_get0(h, db, YB_MODULE, NULL, "/", 0, &td->td_target, NULL, xret)) < 0)
goto done;
if (ret == 0)
goto fail;
/* Clear flags xpath for get */
xml_apply0(td->td_target, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset,
(void*)(XML_FLAG_MARK|XML_FLAG_CHANGE));
/* 2. Parse xml trees
* This is the state we are going from */
if (xmldb_get0(h, "running", YB_MODULE, NULL, "/", 0, &td->td_src, NULL, NULL) < 0)
if ((ret = xmldb_get0(h, "running", YB_MODULE, NULL, "/", 0, &td->td_src, NULL, xret)) < 0)
goto done;
if (ret == 0)
goto fail;
/* Clear flags xpath for get */
xml_apply0(td->td_src, CX_ELMNT, (xml_applyfn_t*)xml_flag_reset,
(void*)(XML_FLAG_MARK|XML_FLAG_CHANGE));
@ -606,11 +609,23 @@ candidate_validate(clicon_handle h,
if ((td = transaction_new()) == NULL)
goto done;
/* Common steps (with commit) */
if ((ret = validate_common(h, db, td, &xret)) < 1){
if ((ret = validate_common(h, db, td, &xret)) < 0){
/* A little complex due to several sources of validation fails or errors.
* (1) xerr is set -> translate to cbret; (2) cbret set use that; otherwise
* use clicon_err. */
if (xret && clicon_xml2cbuf(cbret, xret, 0, 0, -1) < 0)
* use clicon_err.
* TODO: -1 return should be fatal error, not failed validation
*/
if (!cbuf_len(cbret) &&
netconf_operation_failed(cbret, "application", clicon_err_reason)< 0)
goto done;
goto fail;
}
if (ret == 0){
if (xret == NULL){
clicon_err(OE_CFG, EINVAL, "xret is NULL");
goto done;
}
if (clicon_xml2cbuf(cbret, xret, 0, 0, -1) < 0)
goto done;
if (!cbuf_len(cbret) &&
netconf_operation_failed(cbret, "application", clicon_err_reason)< 0)

View file

@ -127,8 +127,7 @@ int netconf_operation_failed(cbuf *cb, char *type, char *message);
int netconf_operation_failed_xml(cxobj **xret, char *type, char *message);
int netconf_malformed_message(cbuf *cb, char *message);
int netconf_malformed_message_xml(cxobj **xret, char *message);
int netconf_data_not_unique_xml(cxobj **xret, cxobj *x, cvec *cvk);
int netconf_data_not_unique_xml(cxobj **xret, cxobj *x, cvec *cvk);
int netconf_minmax_elements_xml(cxobj **xret, cxobj *xp, char *name, int max);
int netconf_trymerge(cxobj *x, yang_stmt *yspec, cxobj **xret);
int netconf_module_features(clicon_handle h);

View file

@ -557,7 +557,7 @@ xmldb_readfile(clicon_handle h,
}
cprintf(cberr, "Internal error: %s", clicon_err_reason);
clicon_err_reset();
if (netconf_operation_failed_xml(xerr, "application", cbuf_get(cberr))< 0)
if (xerr && netconf_operation_failed_xml(xerr, "application", cbuf_get(cberr))< 0)
goto done;
cbuf_free(cberr);
goto fail;

View file

@ -1238,7 +1238,7 @@ _json_parse(char *str,
goto done;
}
cprintf(cberr, "Top-level JSON object %s is not qualified with namespace which is a MUST according to RFC 7951", xml_name(x));
if (netconf_malformed_message_xml(xerr, cbuf_get(cberr)) < 0)
if (xerr && netconf_malformed_message_xml(xerr, cbuf_get(cberr)) < 0)
goto done;
goto fail;
}

View file

@ -131,6 +131,10 @@ netconf_invalid_value_xml(cxobj **xret,
char *encstr = NULL;
cxobj *xa;
if (xret == NULL){
clicon_err(OE_NETCONF, EINVAL, "xret is NULL");
goto done;
}
if (*xret == NULL){
if ((*xret = xml_new("rpc-reply", NULL, CX_ELMNT)) == NULL)
goto done;
@ -248,6 +252,10 @@ netconf_missing_attribute_xml(cxobj **xret,
char *encstr = NULL;
cxobj *xa;
if (xret == NULL){
clicon_err(OE_NETCONF, EINVAL, "xret is NULL");
goto done;
}
if (*xret == NULL){
if ((*xret = xml_new("rpc-reply", NULL, CX_ELMNT)) == NULL)
goto done;
@ -356,6 +364,10 @@ netconf_bad_attribute_xml(cxobj **xret,
char *encstr = NULL;
cxobj *xa;
if (xret == NULL){
clicon_err(OE_NETCONF, EINVAL, "xret is NULL");
goto done;
}
if (*xret == NULL){
if ((*xret = xml_new("rpc-reply", NULL, CX_ELMNT)) == NULL)
goto done;
@ -449,6 +461,10 @@ netconf_common_xml(cxobj **xret,
char *encstr = NULL;
cxobj *xa;
if (xret == NULL){
clicon_err(OE_NETCONF, EINVAL, "xret is NULL");
goto done;
}
if (*xret == NULL){
if ((*xret = xml_new("rpc-reply", NULL, CX_ELMNT)) == NULL)
goto done;
@ -705,6 +721,10 @@ netconf_access_denied_xml(cxobj **xret,
char *encstr = NULL;
cxobj *xa;
if (xret == NULL){
clicon_err(OE_NETCONF, EINVAL, "xret is NULL");
goto done;
}
if (*xret == NULL){
if ((*xret = xml_new("rpc-reply", NULL, CX_ELMNT)) == NULL)
goto done;
@ -941,6 +961,10 @@ netconf_data_missing_xml(cxobj **xret,
cxobj *xerr;
cxobj *xa;
if (xret == NULL){
clicon_err(OE_NETCONF, EINVAL, "xret is NULL");
goto done;
}
if (*xret == NULL){
if ((*xret = xml_new("rpc-reply", NULL, CX_ELMNT)) == NULL)
goto done;
@ -1005,6 +1029,10 @@ netconf_operation_not_supported_xml(cxobj **xret,
char *encstr = NULL;
cxobj *xa;
if (xret == NULL){
clicon_err(OE_NETCONF, EINVAL, "xret is NULL");
goto done;
}
if (*xret == NULL){
if ((*xret = xml_new("rpc-reply", NULL, CX_ELMNT)) == NULL)
goto done;
@ -1116,6 +1144,10 @@ netconf_operation_failed_xml(cxobj **xret,
char *encstr = NULL;
cxobj *xa;
if (xret == NULL){
clicon_err(OE_NETCONF, EINVAL, "xret is NULL");
goto done;
}
if (*xret == NULL){
if ((*xret = xml_new("rpc-reply", NULL, CX_ELMNT)) == NULL)
goto done;
@ -1200,6 +1232,10 @@ netconf_malformed_message_xml(cxobj **xret,
char *encstr = NULL;
cxobj *xa;
if (xret == NULL){
clicon_err(OE_NETCONF, EINVAL, "xret is NULL");
goto done;
}
if (*xret == NULL){
if ((*xret = xml_new("rpc-reply", NULL, CX_ELMNT)) == NULL)
goto done;
@ -1250,8 +1286,12 @@ netconf_data_not_unique_xml(cxobj **xret,
cxobj *xerr;
cxobj *xinfo;
cbuf *cb = NULL;
cxobj *xa;
cxobj *xa;
if (xret == NULL){
clicon_err(OE_NETCONF, EINVAL, "xret is NULL");
goto done;
}
if (*xret == NULL){
if ((*xret = xml_new("rpc-reply", NULL, CX_ELMNT)) == NULL)
goto done;
@ -1316,6 +1356,10 @@ netconf_minmax_elements_xml(cxobj **xret,
cbuf *cb = NULL;
cxobj *xa;
if (xret == NULL){
clicon_err(OE_NETCONF, EINVAL, "xret is NULL");
goto done;
}
if (*xret == NULL){
if ((*xret = xml_new("rpc-reply", NULL, CX_ELMNT)) == NULL)
goto done;
@ -1373,6 +1417,10 @@ netconf_trymerge(cxobj *x,
char *reason = NULL;
cxobj *xc;
if (xret == NULL){
clicon_err(OE_NETCONF, EINVAL, "xret is NULL");
goto done;
}
if (*xret == NULL){
if ((*xret = xml_dup(x)) == NULL)
goto done;

View file

@ -118,7 +118,7 @@ validate_leafref(cxobj *xt,
if ((leafrefbody = xml_body(xt)) == NULL)
goto ok;
if ((ypath = yang_find(ytype, Y_PATH, NULL)) == NULL){
if (netconf_missing_element_xml(xret, "application", yang_argument_get(ytype), "Leafref requires path statement") < 0)
if (xret && netconf_missing_element_xml(xret, "application", yang_argument_get(ytype), "Leafref requires path statement") < 0)
goto done;
goto fail;
}
@ -141,7 +141,7 @@ validate_leafref(cxobj *xt,
goto done;
}
cprintf(cberr, "Leafref validation failed: No leaf %s matching path %s", leafrefbody, path);
if (netconf_bad_element_xml(xret, "application", leafrefbody, cbuf_get(cberr)) < 0)
if (xret && netconf_bad_element_xml(xret, "application", leafrefbody, cbuf_get(cberr)) < 0)
goto done;
goto fail;
}
@ -211,7 +211,7 @@ validate_identityref(cxobj *xt,
/* Get idref value. Then see if this value is derived from ytype.
*/
if ((node = xml_body(xt)) == NULL){ /* It may not be empty */
if (netconf_bad_element_xml(xret, "application", xml_name(xt), "Identityref should not be empty") < 0)
if (xret && netconf_bad_element_xml(xret, "application", xml_name(xt), "Identityref should not be empty") < 0)
goto done;
goto fail;
}
@ -219,13 +219,13 @@ validate_identityref(cxobj *xt,
goto done;
/* This is the type's base reference */
if ((ybaseref = yang_find(ytype, Y_BASE, NULL)) == NULL){
if (netconf_missing_element_xml(xret, "application", yang_argument_get(ytype), "Identityref validation failed, no base") < 0)
if (xret && netconf_missing_element_xml(xret, "application", yang_argument_get(ytype), "Identityref validation failed, no base") < 0)
goto done;
goto fail;
}
/* This is the actual base identity */
if ((ybaseid = yang_find_identity(ybaseref, yang_argument_get(ybaseref))) == NULL){
if (netconf_missing_element_xml(xret, "application", yang_argument_get(ybaseref), "Identityref validation failed, no base identity") < 0)
if (xret && netconf_missing_element_xml(xret, "application", yang_argument_get(ybaseref), "Identityref validation failed, no base identity") < 0)
goto done;
goto fail;
}
@ -254,7 +254,7 @@ validate_identityref(cxobj *xt,
if (cvec_find(idrefvec, idref) == NULL){
cprintf(cberr, "Identityref validation failed, %s not derived from %s",
node, yang_argument_get(ybaseid));
if (netconf_operation_failed_xml(xret, "application", cbuf_get(cberr)) < 0)
if (xret && netconf_operation_failed_xml(xret, "application", cbuf_get(cberr)) < 0)
goto done;
goto fail;
}
@ -337,7 +337,7 @@ xml_yang_validate_rpc(clicon_handle h,
goto done;
/* Only accept resolved NETCONF base namespace */
if (namespace == NULL || strcmp(namespace, NETCONF_BASE_NAMESPACE) != 0){
if (netconf_unknown_namespace_xml(xret, "protocol", rpcprefix, "No appropriate namespace associated with prefix")< 0)
if (xret && netconf_unknown_namespace_xml(xret, "protocol", rpcprefix, "No appropriate namespace associated with prefix")< 0)
goto done;
goto fail;
}
@ -345,7 +345,7 @@ xml_yang_validate_rpc(clicon_handle h,
/* xn is name of rpc, ie <rcp><xn/></rpc> */
while ((xn = xml_child_each(xrpc, xn, CX_ELMNT)) != NULL) {
if ((yn = xml_spec(xn)) == NULL){
if (netconf_unknown_element_xml(xret, "application", xml_name(xn), NULL) < 0)
if (xret && netconf_unknown_element_xml(xret, "application", xml_name(xn), NULL) < 0)
goto done;
goto fail;
}
@ -434,7 +434,7 @@ check_choice(cxobj *xt,
continue; /* not choice */
break;
}
if (netconf_bad_element_xml(xret, "application", xml_name(x), "Element in choice statement already exists") < 0)
if (xret && netconf_bad_element_xml(xret, "application", xml_name(x), "Element in choice statement already exists") < 0)
goto done;
goto fail;
} /* while */
@ -487,7 +487,7 @@ check_list_key(cxobj *xt,
while ((cvi = cvec_each(cvk, cvi)) != NULL) {
keyname = cv_string_get(cvi);
if (xml_find_type(xt, NULL, keyname, CX_ELMNT) == NULL){
if (netconf_missing_element_xml(xret, "application", keyname, "Mandatory key") < 0)
if (xret && netconf_missing_element_xml(xret, "application", keyname, "Mandatory key") < 0)
goto done;
goto fail;
}
@ -557,7 +557,7 @@ check_mandatory(cxobj *xt,
goto done;
}
cprintf(cb, "Mandatory variable of %s in module %s", xml_name(xt), yang_argument_get(ys_module(yc)));
if (netconf_missing_element_xml(xret, "application", yang_argument_get(yc), cbuf_get(cb)) < 0)
if (xret && netconf_missing_element_xml(xret, "application", yang_argument_get(yc), cbuf_get(cb)) < 0)
goto done;
goto fail;
}
@ -574,7 +574,7 @@ check_mandatory(cxobj *xt,
if (x == NULL){
/* @see RFC7950: 15.6 Error Message for Data That Violates
* a Mandatory "choice" Statement */
if (netconf_data_missing_xml(xret, yang_argument_get(yc), NULL) < 0)
if (xret && netconf_data_missing_xml(xret, yang_argument_get(yc), NULL) < 0)
goto done;
goto fail;
}
@ -707,7 +707,7 @@ check_unique_list(cxobj *x,
if (cvi==NULL){
/* Last element (i) is newly inserted, see if it is already there */
if (check_insert_duplicate(vec, i, vlen, sorted) < 0){
if (netconf_data_not_unique_xml(xret, x, cvk) < 0)
if (xret && netconf_data_not_unique_xml(xret, x, cvk) < 0)
goto done;
goto fail;
}
@ -751,7 +751,7 @@ check_min_max(cxobj *xp,
if ((ymin = yang_find(y, Y_MIN_ELEMENTS, NULL)) != NULL){
cv = yang_cv_get(ymin);
if (nr < cv_uint32_get(cv)){
if (netconf_minmax_elements_xml(xret, xp, yang_argument_get(y), 0) < 0)
if (xret && netconf_minmax_elements_xml(xret, xp, yang_argument_get(y), 0) < 0)
goto done;
goto fail;
}
@ -760,7 +760,7 @@ check_min_max(cxobj *xp,
cv = yang_cv_get(ymax);
if (cv_uint32_get(cv) > 0 && /* 0 means unbounded */
nr > cv_uint32_get(cv)){
if (netconf_minmax_elements_xml(xret, xp, yang_argument_get(y), 1) < 0)
if (xret && netconf_minmax_elements_xml(xret, xp, yang_argument_get(y), 1) < 0)
goto done;
goto fail;
}
@ -851,7 +851,7 @@ check_list_unique_minmax(cxobj *xt,
/* Only lists and leaf-lists are allowed to be many
* This checks duplicate container and leafs
*/
if (netconf_minmax_elements_xml(xret, xt, xml_name(x), 1) < 0)
if (xret && netconf_minmax_elements_xml(xret, xt, xml_name(x), 1) < 0)
goto done;
goto fail;
}
@ -1018,14 +1018,14 @@ xml_yang_validate_add(clicon_handle h,
* are considered as "" */
cvtype = cv_type_get(cv);
if (cv_isint(cvtype) || cvtype == CGV_BOOL || cvtype == CGV_DEC64){
if (netconf_bad_element_xml(xret, "application", yang_argument_get(yt), "Invalid NULL value") < 0)
if (xret && netconf_bad_element_xml(xret, "application", yang_argument_get(yt), "Invalid NULL value") < 0)
goto done;
goto fail;
}
}
else{
if (cv_parse1(body, cv, &reason) != 1){
if (netconf_bad_element_xml(xret, "application", yang_argument_get(yt), reason) < 0)
if (xret && netconf_bad_element_xml(xret, "application", yang_argument_get(yt), reason) < 0)
goto done;
goto fail;
}
@ -1033,7 +1033,7 @@ xml_yang_validate_add(clicon_handle h,
if ((ret = ys_cv_validate(h, cv, yt, NULL, &reason)) < 0)
goto done;
if (ret == 0){
if (netconf_bad_element_xml(xret, "application", yang_argument_get(yt), reason) < 0)
if (xret && netconf_bad_element_xml(xret, "application", yang_argument_get(yt), reason) < 0)
goto done;
goto fail;
}
@ -1158,7 +1158,7 @@ xml_yang_validate_all(clicon_handle h,
goto done;
if (ns)
cprintf(cb, " in namespace: %s", ns);
if (netconf_unknown_element_xml(xret, "application", xml_name(xt), cbuf_get(cb)) < 0)
if (xret && netconf_unknown_element_xml(xret, "application", xml_name(xt), cbuf_get(cb)) < 0)
goto done;
goto fail;
}
@ -1217,7 +1217,7 @@ xml_yang_validate_all(clicon_handle h,
}
cprintf(cb, "Failed MUST xpath '%s' of '%s' in module %s",
xpath, xml_name(xt), yang_argument_get(ys_module(ys)));
if (netconf_operation_failed_xml(xret, "application",
if (xret && netconf_operation_failed_xml(xret, "application",
ye?yang_argument_get(ye):cbuf_get(cb)) < 0)
goto done;
goto fail;
@ -1247,7 +1247,7 @@ xml_yang_validate_all(clicon_handle h,
cprintf(cb, "Failed WHEN condition of %s in module %s",
xml_name(xt),
yang_argument_get(ys_module(ys)));
if (netconf_operation_failed_xml(xret, "application",
if (xret && netconf_operation_failed_xml(xret, "application",
cbuf_get(cb)) < 0)
goto done;
goto fail;
@ -1269,7 +1269,7 @@ xml_yang_validate_all(clicon_handle h,
xpath,
xml_name(xt),
yang_argument_get(ys_module(ys)));
if (netconf_operation_failed_xml(xret, "application",
if (xret && netconf_operation_failed_xml(xret, "application",
cbuf_get(cb)) < 0)
goto done;
goto fail;

View file

@ -329,7 +329,7 @@ yang_modules_state_get(clicon_handle h,
* Note, list is not sorted since it is state (should not be)
*/
if (clixon_xml_parse_string(cbuf_get(cb), YB_MODULE, yspec, &x, NULL) < 0){
if (netconf_operation_failed_xml(xret, "protocol", clicon_err_reason)< 0)
if (xret && netconf_operation_failed_xml(xret, "protocol", clicon_err_reason)< 0)
goto done;
goto fail;
}