experiments/clixon
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

* Bundle internal NETCONF on RESTCONF: A RESTCONF operation could produce several (up to four) internal NETCONF messages between RESTCONF server and backend. These have now been bundled into one.

Browse source 
* NACM recovery user session is now properly enforced. This means that if `CLICON_NACM_CREDENTIALS` is `except` (default), then a specific `CLICON_NACM_RECOVERY_USER` can make any edits and bypass NACM rules.
* If a default value is replaced by an actual value, RESTCONF return values have changed from `204 No Content` to `201 Created`
* clixon-config.yang: Removed default valude of CLICON_NACM_RECOVERY_USER
This commit is contained in:
Olof hagsand 2020-08-11 14:46:53 +02:00
parent de85f20415
commit dc1ad560f9
12 changed files with 217 additions and 261 deletions
Download patch file Download diff file Expand all files Collapse all files

17
CHANGELOG.md
View file

@ -46,6 +46,7 @@ Expected: July 2020
* Enforcing RFC 7950 Sec 7.6.1 means unassigned top-level leafs (or leafs under non-presence containers) are assigned default values. * Enforcing RFC 7950 Sec 7.6.1 means unassigned top-level leafs (or leafs under non-presence containers) are assigned default values.
* In this process non-presence containers may be created. * In this process non-presence containers may be created.
* See also [default values don't show up in datastores #111](https://github.com/clicon/clixon/issues/111). * See also [default values don't show up in datastores #111](https://github.com/clicon/clixon/issues/111).
* If a default value is replaced by an actual value, RESTCONF return values have changed from `204 No Content` to `201 Created`
* NACM default behaviour is read-only (empty configs are dead-lockedd) * NACM default behaviour is read-only (empty configs are dead-lockedd)
* This applies if NACM is loaded and `CLICON_NACM_MODE` is `internal` * This applies if NACM is loaded and `CLICON_NACM_MODE` is `internal`
* Due to the previous bult (top-level default leafs) * Due to the previous bult (top-level default leafs)
@ -54,6 +55,11 @@ Expected: July 2020
1. Access the system with the recovery user, see clixon option CLICON_NACM_RECOVERY_USER 1. Access the system with the recovery user, see clixon option CLICON_NACM_RECOVERY_USER
2. Edit the startup-db with a valid NACM config and restart the system 2. Edit the startup-db with a valid NACM config and restart the system
3. Set clixon option CLICON_NACM_DISABLED_ON_EMPTY to true which means that if the config is (completely) empty, you can add a NACM config as a first edit. 3. Set clixon option CLICON_NACM_DISABLED_ON_EMPTY to true which means that if the config is (completely) empty, you can add a NACM config as a first edit.
* NACM recovery user session is now properly enforced. This means that if `CLICON_NACM_CREDENTIALS` is `except` (default), then a specific `CLICON_NACM_RECOVERY_USER` can make any edits and bypass NACM rules.
* Either this user must exist as UNIX user and be logged in by the client (eg CLI/NETCONF), or
* The client is "trusted" (root/wwwuser) and the recovery user is used as a pseudo-user when accessing the backend.
* For Restconf using proper authentication (eg SSL client certs) the recovery user must be an authenticated client.
* Netconf lock/unlock behaviour changed to adhere to RFC 6241 * Netconf lock/unlock behaviour changed to adhere to RFC 6241
* Changed commit lock error tag from "lock denied" to "in-use". * Changed commit lock error tag from "lock denied" to "in-use".
* Changed unlock error message from "lock is already held" to #lock not active" or "lock held by other session". * Changed unlock error message from "lock is already held" to #lock not active" or "lock held by other session".
@ -66,6 +72,7 @@ Expected: July 2020
* CLICON_SSL_SERVER_KEY * CLICON_SSL_SERVER_KEY
* CLICON_SSL_CA_CERT * CLICON_SSL_CA_CERT
* Added CLICON_NACM_DISABLED_ON_EMPTY to mitigate read-only "dead-lock" of empty startup configs. * Added CLICON_NACM_DISABLED_ON_EMPTY to mitigate read-only "dead-lock" of empty startup configs.
* Removed default valude of CLICON_NACM_RECOVERY_USER
* Restconf FCGI (eg via nginx) have changed reply message syntax slightly as follows (due to refactoring and common code with evhtp): * Restconf FCGI (eg via nginx) have changed reply message syntax slightly as follows (due to refactoring and common code with evhtp):
* Bodies in error retuns including html code have been removed * Bodies in error retuns including html code have been removed
* Some (extra) CRLF:s have been removed * Some (extra) CRLF:s have been removed
@ -100,6 +107,15 @@ Expected: July 2020
### Minor changes ### Minor changes
* Bundle internal NETCONF on RESTCONF
* A RESTCONF operation could produce several (up to four) internal NETCONF messages between RESTCONF server and backend. These have now been bundled into one.
* Added several extensions to clixon NETCONF to carry information between RESTCONF client and backend. This includes several attributes:
* autocommit - perform a operation immediately after an edit.
* copystartup - copy the running db to the startup db after a edit/ commit.
* objectcreate/objectexisted - PATCH and PUT create/merge behaviour is not consistent with vanilla NETCONF operations, these attributes carry more information to make them. In particular:
* RFC 8040 4.5 PUT: if the PUT request creates a new resource, a "201 Created" status-line is returned. If an existing resource is modified, a "204 No Content" status-line is returned.
* RFC 8040 4.6 PATCH: If the target resource instance does not exist, the server MUST NOT create it.
* Improved performance, especially latency.
* New backend switch: `-q` : Quit startup directly after upgrading and print result on stdout. * New backend switch: `-q` : Quit startup directly after upgrading and print result on stdout.
* Enhanced Clixon if-feature handling: * Enhanced Clixon if-feature handling:
* If-feature now supports and/or lists, such as: `if-feature "a and b"` and `if-feature "a or b or c"`. However, full if-feature-expr including `not` and nested boolean experessions is still not supported. * If-feature now supports and/or lists, such as: `if-feature "a and b"` and `if-feature "a or b or c"`. However, full if-feature-expr including `not` and nested boolean experessions is still not supported.
@ -112,7 +128,6 @@ Expected: July 2020
### Corrected Bugs ### Corrected Bugs
* Changed (restricted) recovery NACM user session.
* Fixed: [default values don't show up in datastores #111](https://github.com/clicon/clixon/issues/111). * Fixed: [default values don't show up in datastores #111](https://github.com/clicon/clixon/issues/111).
* See also API changes since this changes NACM behavior for example. * See also API changes since this changes NACM behavior for example.
* Fixed: Don't call upgrade callbacks if no revision defined so there's no way to determine right way 'from' and 'to' * Fixed: Don't call upgrade callbacks if no revision defined so there's no way to determine right way 'from' and 'to'

41
apps/backend/backend_client.c
View file

@ -576,6 +576,9 @@ from_client_edit_config(clicon_handle h,
int ret; int ret;
char *username; char *username;
cxobj *xret = NULL; cxobj *xret = NULL;
char *attr;
int autocommit = 0;
char *val = NULL;
username = clicon_username_get(h); username = clicon_username_get(h);
if ((yspec = clicon_dbspec_yang(h)) == NULL){ if ((yspec = clicon_dbspec_yang(h)) == NULL){
@ -605,7 +608,6 @@ from_client_edit_config(clicon_handle h,
goto done; goto done;
goto ok; goto ok;
} }
if ((x = xpath_first(xn, NULL, "default-operation")) != NULL){ if ((x = xpath_first(xn, NULL, "default-operation")) != NULL){
if (xml_operation(xml_body(x), &operation) < 0){ if (xml_operation(xml_body(x), &operation) < 0){
if (netconf_invalid_value(cbret, "protocol", "Wrong operation")< 0) if (netconf_invalid_value(cbret, "protocol", "Wrong operation")< 0)
@ -660,8 +662,41 @@ from_client_edit_config(clicon_handle h,
if (ret == 0) if (ret == 0)
goto ok; goto ok;
xmldb_modified_set(h, target, 1); /* mark as dirty */ xmldb_modified_set(h, target, 1); /* mark as dirty */
/* Clixon extension: autocommit */
if ((attr = xml_find_value(xn, "autocommit")) != NULL &&
strcmp(attr,"true")==0)
autocommit = 1;
/* If autocommit option is set or requested by client */
if (clicon_autocommit(h) || autocommit) {
if ((ret = candidate_commit(h, "candidate", cbret)) < 0){ /* Assume validation fail, nofatal */
if (ret < 0)
if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0)
goto done;
goto ok;
}
if (ret == 0){ /* discard */
if (xmldb_copy(h, "running", "candidate") < 0){
if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0)
goto done;
goto ok;
}
goto ok;
}
}
/* Clixon extension: copy */
if ((attr = xml_find_value(xn, "copystartup")) != NULL &&
strcmp(attr,"true") == 0){
if (xmldb_copy(h, "running", "startup") < 0){
if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0)
goto done;
goto ok;
}
}
assert(cbuf_len(cbret) == 0); assert(cbuf_len(cbret) == 0);
cprintf(cbret, "<rpc-reply><ok/></rpc-reply>"); cprintf(cbret, "<rpc-reply><ok");
if (clicon_data_get(h, "objectexisted", &val) == 0)
cprintf(cbret, " objectexisted=\"%s\"", val);
cprintf(cbret, "/></rpc-reply>");
ok: ok:
retval = 0; retval = 0;
done: done:
@ -1026,7 +1061,7 @@ from_client_get(clicon_handle h,
content = netconf_content_str2int(attr); content = netconf_content_str2int(attr);
/* Clixon extensions: depth */ /* Clixon extensions: depth */
if ((attr = xml_find_value(xe, "depth")) != NULL){ if ((attr = xml_find_value(xe, "depth")) != NULL){
char *reason = NULL; char *reason = NULL;
if ((ret = parse_int32(attr, &depth, &reason)) < 0){ if ((ret = parse_int32(attr, &depth, &reason)) < 0){
clicon_err(OE_XML, errno, "parse_int32"); clicon_err(OE_XML, errno, "parse_int32");
goto done; goto done;

4
apps/cli/cli_common.c
View file

@ -312,10 +312,6 @@ cli_dbxml(clicon_handle h,
goto done; goto done;
if (clicon_rpc_edit_config(h, "candidate", OP_NONE, cbuf_get(cb)) < 0) if (clicon_rpc_edit_config(h, "candidate", OP_NONE, cbuf_get(cb)) < 0)
goto done; goto done;
if (clicon_autocommit(h)) {
if (clicon_rpc_commit(h) < 0)
goto done;
}
retval = 0; retval = 0;
done: done:
if (xerr) if (xerr)

3
apps/restconf/restconf_lib.c
View file

@ -258,7 +258,8 @@ restconf_terminate(clicon_handle h)
return 0; return 0;
} }
/*! If restconf insert/point attributes are present, translate to netconf /*! If RESTCONF insert/point attributes are present, translate to NETCONF
*
* @param[in] xdata URI->XML to translate * @param[in] xdata URI->XML to translate
* @param[in] qvec Query parameters (eg where insert/point should be) * @param[in] qvec Query parameters (eg where insert/point should be)
* @retval 0 OK * @retval 0 OK

183
apps/restconf/restconf_methods.c
View file

@ -272,7 +272,8 @@ api_data_write(clicon_handle h,
cvec *nsc = NULL; cvec *nsc = NULL;
yang_bind yb; yang_bind yb;
char *xpath = NULL; char *xpath = NULL;
char *attr;
clicon_debug(1, "%s api_path:\"%s\"", __FUNCTION__, api_path0); clicon_debug(1, "%s api_path:\"%s\"", __FUNCTION__, api_path0);
clicon_debug(1, "%s data:\"%s\"", __FUNCTION__, data); clicon_debug(1, "%s data:\"%s\"", __FUNCTION__, data);
if ((yspec = clicon_dbspec_yang(h)) == NULL){ if ((yspec = clicon_dbspec_yang(h)) == NULL){
@ -297,43 +298,10 @@ api_data_write(clicon_handle h,
goto ok; goto ok;
} }
} }
if (plain_patch)
xret = NULL; op = OP_MERGE; /* bad request if it does not exist */
if (clicon_rpc_get_config(h, clicon_nacm_recovery_user(h), else
"candidate", xpath, nsc, &xret) < 0){ op = OP_REPLACE; /* OP_CREATE if it does not exist */
if (netconf_operation_failed_xml(&xerr, "protocol", clicon_err_reason) < 0)
goto done;
if ((xe = xpath_first(xerr, NULL, "rpc-error")) == NULL){
clicon_err(OE_XML, EINVAL, "rpc-error not found (internal error)");
goto done;
}
if (api_return_err(h, req, xe, pretty, media_out, 0) < 0)
goto done;
goto ok;
}
#if 0
if (clicon_debug_get())
clicon_log_xml(LOG_DEBUG, xret, "%s xret:", __FUNCTION__);
#endif
if (xml_child_nr(xret) == 0){ /* Object does not exist */
if (plain_patch){ /* If the target resource instance does not exist, the server MUST NOT create it. */
restconf_badrequest(h, req);
goto ok;
}
else
op = OP_CREATE;
}
else{
if (plain_patch)
op = OP_MERGE;
else
op = OP_REPLACE;
}
if (xret){
xml_free(xret);
xret = NULL;
}
/* Create config top-of-tree */ /* Create config top-of-tree */
if ((xtop = xml_new("config", NULL, CX_ELMNT)) == NULL) if ((xtop = xml_new("config", NULL, CX_ELMNT)) == NULL)
goto done; goto done;
@ -491,9 +459,20 @@ api_data_write(clicon_handle h,
goto done; goto done;
if (xml_prefix_set(xa, NETCONF_BASE_PREFIX) < 0) if (xml_prefix_set(xa, NETCONF_BASE_PREFIX) < 0)
goto done; goto done;
if (xml_value_set(xa, xml_operation2str(op)) < 0) if (xml_value_set(xa, xml_operation2str(op)) < 0) /* XXX here is where op is used */
goto done; goto done;
if ((xa = xml_new("objectcreate", xdata, CX_ATTR)) == NULL)
goto done;
if (plain_patch){
/* RFC 8040 4.6. PATCH:
* If the target resource instance does not exist, the server MUST NOT create it.
*/
if (xml_value_set(xa, "false") < 0)
goto done;
}
else
if (xml_value_set(xa, "true") < 0)
goto done;
/* Top-of tree, no api-path /* Top-of tree, no api-path
* Replace xparent with x, ie bottom of api-path with data * Replace xparent with x, ie bottom of api-path with data
*/ */
@ -608,7 +587,17 @@ api_data_write(clicon_handle h,
username?username:"", username?username:"",
NETCONF_BASE_PREFIX, NETCONF_BASE_PREFIX,
NETCONF_BASE_NAMESPACE); /* bind nc to netconf namespace */ NETCONF_BASE_NAMESPACE); /* bind nc to netconf namespace */
cprintf(cbx, "<edit-config><target><candidate /></target>"); cprintf(cbx, "<edit-config");
/* RFC8040 Sec 1.4:
* If the NETCONF server supports :startup, the RESTCONF server MUST
* automatically update the non-volatile startup configuration
* datastore, after the "running" datastore has been altered as a
* consequence of a RESTCONF edit operation.
*/
if (if_feature(yspec, "ietf-netconf", "startup"))
cprintf(cbx, " copystartup=\"true\"");
cprintf(cbx, " autocommit=\"true\"");
cprintf(cbx, "><target><candidate /></target>");
cprintf(cbx, "<default-operation>none</default-operation>"); cprintf(cbx, "<default-operation>none</default-operation>");
if (clicon_xml2cbuf(cbx, xtop, 0, 0, -1) < 0) if (clicon_xml2cbuf(cbx, xtop, 0, 0, -1) < 0)
goto done; goto done;
@ -621,58 +610,15 @@ api_data_write(clicon_handle h,
goto done; goto done;
goto ok; goto ok;
} }
cbuf_reset(cbx); if ((xe = xpath_first(xret, NULL, "//ok")) != NULL &&
/* commit/discard should be done automaticaly by the system, therefore (attr = xml_find_value(xe, "objectexisted")) != NULL &&
* recovery user is used here (edit-config but not commit may be permitted strcmp(attr, "false")==0){
by NACM */ if (restconf_reply_send(req, 201, NULL) < 0) /* Created */
cprintf(cbx, "<rpc username=\"%s\">", clicon_nacm_recovery_user(h));
cprintf(cbx, "<commit/></rpc>");
if (clicon_rpc_netconf(h, cbuf_get(cbx), &xretcom, NULL) < 0)
goto done;
if ((xe = xpath_first(xretcom, NULL, "//rpc-error")) != NULL){
cbuf_reset(cbx);
cprintf(cbx, "<rpc username=\"%s\">", username?username:"");
cprintf(cbx, "<discard-changes/></rpc>");
if (clicon_rpc_netconf(h, cbuf_get(cbx), &xretdis, NULL) < 0)
goto done; goto done;
/* log errors from discard, but ignore */
if ((xpath_first(xretdis, NULL, "//rpc-error")) != NULL)
clicon_log(LOG_WARNING, "%s: discard-changes failed which may lead candidate in an inconsistent state", __FUNCTION__);
if (api_return_err(h, req, xe, pretty, media_out, 0) < 0)
goto done;
goto ok;
} }
if (xretcom){ /* Clear: can be reused again below */ else
xml_free(xretcom); if (restconf_reply_send(req, 204, NULL) < 0) /* No content */
xretcom = NULL;
}
if (if_feature(yspec, "ietf-netconf", "startup")){
/* RFC8040 Sec 1.4:
* If the NETCONF server supports :startup, the RESTCONF server MUST
* automatically update the non-volatile startup configuration
* datastore, after the "running" datastore has been altered as a
* consequence of a RESTCONF edit operation.
*/
cbuf_reset(cbx);
cprintf(cbx, "<rpc username=\"%s\">", clicon_nacm_recovery_user(h));
cprintf(cbx, "<copy-config><source><running/></source><target><startup/></target></copy-config></rpc>");
if (clicon_rpc_netconf(h, cbuf_get(cbx), &xretcom, NULL) < 0)
goto done;
/* If copy-config failed, log and ignore (already committed) */
if ((xe = xpath_first(xretcom, NULL, "//rpc-error")) != NULL){
clicon_log(LOG_WARNING, "%s: copy-config running->startup failed", __FUNCTION__);
}
}
/* Check if it was created, or if we tried again and replaced it */
if (op == OP_CREATE){
if (restconf_reply_send(req, 201, NULL) < 0)
goto done; goto done;
}
else{
if (restconf_reply_send(req, 204, NULL) < 0)
goto done;
}
ok: ok:
retval = 0; retval = 0;
done: done:
@ -877,7 +823,18 @@ api_data_delete(clicon_handle h,
username?username:"", username?username:"",
NETCONF_BASE_PREFIX, NETCONF_BASE_PREFIX,
NETCONF_BASE_NAMESPACE); /* bind nc to netconf namespace */ NETCONF_BASE_NAMESPACE); /* bind nc to netconf namespace */
cprintf(cbx, "<edit-config><target><candidate /></target>");
cprintf(cbx, "<edit-config");
/* RFC8040 Sec 1.4:
* If the NETCONF server supports :startup, the RESTCONF server MUST
* automatically update the non-volatile startup configuration
* datastore, after the "running" datastore has been altered as a
* consequence of a RESTCONF edit operation.
*/
if (if_feature(yspec, "ietf-netconf", "startup"))
cprintf(cbx, " copystartup=\"true\"");
cprintf(cbx, " autocommit=\"true\"");
cprintf(cbx, "><target><candidate /></target>");
cprintf(cbx, "<default-operation>none</default-operation>"); cprintf(cbx, "<default-operation>none</default-operation>");
if (clicon_xml2cbuf(cbx, xtop, 0, 0, -1) < 0) if (clicon_xml2cbuf(cbx, xtop, 0, 0, -1) < 0)
goto done; goto done;
@ -889,50 +846,6 @@ api_data_delete(clicon_handle h,
goto done; goto done;
goto ok; goto ok;
} }
/* Assume this is validation failed since commit includes validate */
cbuf_reset(cbx);
/* commit/discard should be done automatically by the system, therefore
* recovery user is used here (edit-config but not commit may be permitted
by NACM */
cprintf(cbx, "<rpc username=\"%s\">", clicon_nacm_recovery_user(h));
cprintf(cbx, "<commit/></rpc>");
if (clicon_rpc_netconf(h, cbuf_get(cbx), &xretcom, NULL) < 0)
goto done;
if ((xe = xpath_first(xretcom, NULL, "//rpc-error")) != NULL){
cbuf_reset(cbx);
cprintf(cbx, "<rpc username=\"%s\">", clicon_nacm_recovery_user(h));
cprintf(cbx, "<discard-changes/></rpc>");
if (clicon_rpc_netconf(h, cbuf_get(cbx), &xretdis, NULL) < 0)
goto done;
/* log errors from discard, but ignore */
if ((xpath_first(xretdis, NULL, "//rpc-error")) != NULL)
clicon_log(LOG_WARNING, "%s: discard-changes failed which may lead candidate in an inconsistent state", __FUNCTION__);
if (api_return_err(h, req, xe, pretty, media_out, 0) < 0)
goto done;
goto ok;
}
if (xretcom){ /* Clear: can be reused again below */
xml_free(xretcom);
xretcom = NULL;
}
if (if_feature(yspec, "ietf-netconf", "startup")){
/* RFC8040 Sec 1.4:
* If the NETCONF server supports :startup, the RESTCONF server MUST
* automatically update the non-volatile startup configuration
* datastore, after the "running" datastore has been altered as a
* consequence of a RESTCONF edit operation.
*/
cbuf_reset(cbx);
cprintf(cbx, "<rpc username=\"%s\">", clicon_nacm_recovery_user(h));
cprintf(cbx, "<copy-config><source><running/></source><target><startup/></target></copy-config></rpc>");
if (clicon_rpc_netconf(h, cbuf_get(cbx), &xretcom, NULL) < 0)
goto done;
/* If copy-config failed, log and ignore (already committed) */
if ((xe = xpath_first(xretcom, NULL, "//rpc-error")) != NULL){
clicon_log(LOG_WARNING, "%s: copy-config running->startup failed", __FUNCTION__);
}
}
if (restconf_reply_send(req, 204, NULL) < 0) if (restconf_reply_send(req, 204, NULL) < 0)
goto done; goto done;
ok: ok:

56
apps/restconf/restconf_methods_post.c
View file

@ -370,7 +370,18 @@ api_data_post(clicon_handle h,
username?username:"", username?username:"",
NETCONF_BASE_PREFIX, NETCONF_BASE_PREFIX,
NETCONF_BASE_NAMESPACE); /* bind nc to netconf namespace */ NETCONF_BASE_NAMESPACE); /* bind nc to netconf namespace */
cprintf(cbx, "<edit-config><target><candidate /></target>");
cprintf(cbx, "<edit-config");
/* RFC8040 Sec 1.4:
* If the NETCONF server supports :startup, the RESTCONF server MUST
* automatically update the non-volatile startup configuration
* datastore, after the "running" datastore has been altered as a
* consequence of a RESTCONF edit operation.
*/
if (if_feature(yspec, "ietf-netconf", "startup"))
cprintf(cbx, " copystartup=\"true\"");
cprintf(cbx, " autocommit=\"true\"");
cprintf(cbx, "><target><candidate /></target>");
cprintf(cbx, "<default-operation>none</default-operation>"); cprintf(cbx, "<default-operation>none</default-operation>");
if (clicon_xml2cbuf(cbx, xtop, 0, 0, -1) < 0) if (clicon_xml2cbuf(cbx, xtop, 0, 0, -1) < 0)
goto done; goto done;
@ -383,49 +394,6 @@ api_data_post(clicon_handle h,
goto done; goto done;
goto ok; goto ok;
} }
/* Assume this is validation failed since commit includes validate */
cbuf_reset(cbx);
/* commit/discard should be done automaticaly by the system, therefore
* recovery user is used here (edit-config but not commit may be permitted
by NACM */
cprintf(cbx, "<rpc username=\"%s\">", clicon_nacm_recovery_user(h));
cprintf(cbx, "<commit/></rpc>");
if (clicon_rpc_netconf(h, cbuf_get(cbx), &xretcom, NULL) < 0)
goto done;
if ((xe = xpath_first(xretcom, NULL, "//rpc-error")) != NULL){
cbuf_reset(cbx);
cprintf(cbx, "<rpc username=\"%s\">", username?username:"");
cprintf(cbx, "<discard-changes/></rpc>");
if (clicon_rpc_netconf(h, cbuf_get(cbx), &xretdis, NULL) < 0)
goto done;
/* log errors from discard, but ignore */
if ((xpath_first(xretdis, NULL, "//rpc-error")) != NULL)
clicon_log(LOG_WARNING, "%s: discard-changes failed which may lead candidate in an inconsistent state", __FUNCTION__);
if (api_return_err(h, req, xe, pretty, media_out, 0) < 0) /* Use original xe */
goto done;
goto ok;
}
if (xretcom){ /* Clear: can be reused again below */
xml_free(xretcom);
xretcom = NULL;
}
if (if_feature(yspec, "ietf-netconf", "startup")){
/* RFC8040 Sec 1.4:
* If the NETCONF server supports :startup, the RESTCONF server MUST
* automatically update the non-volatile startup configuration
* datastore, after the "running" datastore has been altered as a
* consequence of a RESTCONF edit operation.
*/
cbuf_reset(cbx);
cprintf(cbx, "<rpc username=\"%s\">", clicon_nacm_recovery_user(h));
cprintf(cbx, "<copy-config><source><running/></source><target><startup/></target></copy-config></rpc>");
if (clicon_rpc_netconf(h, cbuf_get(cbx), &xretcom, NULL) < 0)
goto done;
/* If copy-config failed, log and ignore (already committed) */
if ((xe = xpath_first(xretcom, NULL, "//rpc-error")) != NULL){
clicon_log(LOG_WARNING, "%s: copy-config running->startup failed", __FUNCTION__);
}
}
if (http_location_header(h, req, xdata) < 0) if (http_location_header(h, req, xdata) < 0)
goto done; goto done;
if (restconf_reply_send(req, 201, NULL) < 0) if (restconf_reply_send(req, 201, NULL) < 0)

2
lib/src/clixon_data.c
View file

@ -83,7 +83,7 @@
* @param[in] name Data name * @param[in] name Data name
* @param[out] val Data value as string * @param[out] val Data value as string
* @retval 0 OK * @retval 0 OK
* @retval -1 Not found * @retval -1 Not found (or error)
* @see clicon_option_str * @see clicon_option_str
*/ */
int int

49
lib/src/clixon_datastore_write.c
View file

@ -119,7 +119,8 @@ attr_ns_value(cxobj *x,
goto fail; goto fail;
} }
/* the attribute exists, but not w expected namespace */ /* the attribute exists, but not w expected namespace */
if (strcmp(ans, ns) == 0) if (ns == NULL ||
strcmp(ans, ns) == 0)
val = xml_value(xa); val = xml_value(xa);
} }
*valp = val; *valp = val;
@ -257,10 +258,14 @@ text_modify(clicon_handle h,
enum insert_type insert = INS_LAST; enum insert_type insert = INS_LAST;
int changed = 0; /* Only if x0p's children have changed-> sort necessary */ int changed = 0; /* Only if x0p's children have changed-> sort necessary */
cvec *nscx1 = NULL; cvec *nscx1 = NULL;
char *createstr = NULL;
if (x1 == NULL){
clicon_err(OE_XML, EINVAL, "x1 is missing");
goto done;
}
/* Check for operations embedded in tree according to netconf */ /* Check for operations embedded in tree according to netconf */
if ((ret = attr_ns_value(x1, if ((ret = attr_ns_value(x1, "operation", NETCONF_BASE_NAMESPACE,
"operation", NETCONF_BASE_NAMESPACE,
cbret, &opstr)) < 0) cbret, &opstr)) < 0)
goto done; goto done;
if (ret == 0) if (ret == 0)
@ -268,6 +273,28 @@ text_modify(clicon_handle h,
if (opstr != NULL) if (opstr != NULL)
if (xml_operation(opstr, &op) < 0) if (xml_operation(opstr, &op) < 0)
goto done; goto done;
if ((ret = attr_ns_value(x1, "objectcreate", NULL, cbret, &createstr)) < 0)
goto done;
if (ret == 0)
goto fail;
if (createstr != NULL &&
(op == OP_REPLACE || op == OP_MERGE || op == OP_CREATE)){
if (x0 == NULL || xml_nopresence_default(x0)){ /* does not exist or is default */
if (strcmp(createstr, "false")==0){
/* RFC 8040 4.6 PATCH:
* If the target resource instance does not exist, the server MUST NOT create it.
*/
if (netconf_data_missing(cbret, NULL,
"RFC 8040 4.6. PATCH: If the target resource instance does not exist, the server MUST NOT create it") < 0)
goto done;
goto fail;
}
clicon_data_set(h, "objectexisted", "false");
}
else{ /* exists */
clicon_data_set(h, "objectexisted", "true");
}
}
x1name = xml_name(x1); x1name = xml_name(x1);
if (yang_keyword_get(y0) == Y_LEAF_LIST || if (yang_keyword_get(y0) == Y_LEAF_LIST ||
yang_keyword_get(y0) == Y_LEAF){ yang_keyword_get(y0) == Y_LEAF){
@ -708,7 +735,8 @@ text_modify_top(clicon_handle h,
yang_stmt *ymod;/* yang module */ yang_stmt *ymod;/* yang module */
char *opstr; char *opstr;
int ret; int ret;
char *createstr = NULL;
/* Check for operations embedded in tree according to netconf */ /* Check for operations embedded in tree according to netconf */
if ((ret = attr_ns_value(x1, if ((ret = attr_ns_value(x1,
"operation", NETCONF_BASE_NAMESPACE, "operation", NETCONF_BASE_NAMESPACE,
@ -719,7 +747,11 @@ text_modify_top(clicon_handle h,
if (opstr != NULL) if (opstr != NULL)
if (xml_operation(opstr, &op) < 0) if (xml_operation(opstr, &op) < 0)
goto done; goto done;
/* Special case if x1 is empty, top-level only <config/> */ if ((ret = attr_ns_value(x1, "objectcreate", NULL, cbret, &createstr)) < 0)
goto done;
if (ret == 0)
goto fail;
/* Special case if incoming x1 is empty, top-level only <config/> */
if (xml_child_nr_type(x1, CX_ELMNT) == 0){ if (xml_child_nr_type(x1, CX_ELMNT) == 0){
if (xml_child_nr_type(x0, CX_ELMNT)){ /* base tree not empty */ if (xml_child_nr_type(x0, CX_ELMNT)){ /* base tree not empty */
switch(op){ switch(op){
@ -760,6 +792,12 @@ text_modify_top(clicon_handle h,
} }
/* Special case top-level replace */ /* Special case top-level replace */
else if (op == OP_REPLACE || op == OP_DELETE){ else if (op == OP_REPLACE || op == OP_DELETE){
if (createstr != NULL){
if (xml_child_nr_type(x0, CX_ELMNT)) /* base tree not empty */
clicon_data_set(h, "objectexisted", "true");
else
clicon_data_set(h, "objectexisted", "false");
}
if (!permit && xnacm){ if (!permit && xnacm){
if ((ret = nacm_datanode_write(h, x1, x1t, NACM_UPDATE, username, xnacm, cbret)) < 0) if ((ret = nacm_datanode_write(h, x1, x1t, NACM_UPDATE, username, xnacm, cbret)) < 0)
goto done; goto done;
@ -951,6 +989,7 @@ xmldb_put(clicon_handle h,
permit = (xnacm==NULL); permit = (xnacm==NULL);
/* Here assume if xnacm is set and !permit do NACM */ /* Here assume if xnacm is set and !permit do NACM */
clicon_data_del(h, "objectexisted");
/* /*
* Modify base tree x with modification x1. This is where the * Modify base tree x with modification x1. This is where the
* new tree is made. * new tree is made.

2
lib/src/clixon_netconf_lib.c
View file

@ -838,7 +838,7 @@ netconf_data_exists(cbuf *cb,
* does not exist. For example, a "delete" operation was attempted on * does not exist. For example, a "delete" operation was attempted on
* data that does not exist. * data that does not exist.
* @param[out] cb CLIgen buf. Error XML is written in this buffer * @param[out] cb CLIgen buf. Error XML is written in this buffer
* @param[in] missing_choice If set, see RFC7950: 15.6 violates mandatiry choice * @param[in] missing_choice If set, see RFC7950: 15.6 violates mandatory choice
* @param[in] message Error message * @param[in] message Error message
*/ */
int int

2
test/test_nacm_protocol.sh
View file

@ -210,7 +210,7 @@ new "commit it"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><commit/></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><commit/></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
new "enable nacm" new "enable nacm"
expectpart "$(curl -u andy:bar $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" -d '{"ietf-netconf-acm:enable-nacm": true}' $RCPROTO://localhost/restconf/data/ietf-netconf-acm:nacm/enable-nacm)" 0 "HTTP/1.1 204 No Content" expectpart "$(curl -u andy:bar $CURLOPTS -X PUT -H "Content-Type: application/yang-data+json" -d '{"ietf-netconf-acm:enable-nacm": true}' $RCPROTO://localhost/restconf/data/ietf-netconf-acm:nacm/enable-nacm)" 0 "HTTP/1.1 201 Created"
# Rule 3: permit-edit-config # Rule 3: permit-edit-config
new "permit-edit-config: limited ok restconf" new "permit-edit-config: limited ok restconf"

115
test/test_nacm_recovery.sh
View file

@ -4,7 +4,7 @@
# the config even though NACM is enabled and write is DENY # the config even though NACM is enabled and write is DENY
# Only use netconf - restconf also has authentication on web level, and that gets # Only use netconf - restconf also has authentication on web level, and that gets
# another layer # another layer
# The only recovery session that work are: (last true arg to testrun) # Main test default except mode, it gets too complicated otherwise
# #
# Magic line must be first in script (see README.md) # Magic line must be first in script (see README.md)
@ -148,89 +148,78 @@ EOF
fi fi
} }
#------- REALUSER: $USER #------- CRED: except USER: non-root
# This is default, therefore first
# Neither of these should work: user != recovery CRED=except
REALUSER=$USER REALUSER=$USER
# Recovery as a seperate user does not work
PSEUDO=$USER PSEUDO=$USER
RECOVERY=_recovery RECOVERY=_recovery
for c in none exact except; do new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" testrun $CRED $REALUSER $PSEUDO $RECOVERY true false
testrun $c $REALUSER $PSEUDO $RECOVERY true false
done
# All these should work: user == recovery # Recovery as actual user works
REALUSER=$USER
PSEUDO=$USER PSEUDO=$USER
RECOVERY=$USER RECOVERY=$USER
for c in none exact except; do new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" testrun $CRED $REALUSER $PSEUDO $RECOVERY true true
testrun $c $REALUSER $PSEUDO $RECOVERY true true
done
# Only none credentials should work # pseudo-user as recovery user does not work if actual user is non-root/non-web
REALUSER=$USER
PSEUDO=_recovery PSEUDO=_recovery
RECOVERY=_recovery RECOVERY=_recovery
new "cred: none realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
testrun none $REALUSER $PSEUDO $RECOVERY true true testrun $CRED $REALUSER $PSEUDO $RECOVERY false false
for c in exact except; do
new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
testrun $c $REALUSER $PSEUDO $RECOVERY false false
done
# None of these work
REALUSER=$USER
PSEUDO=_recovery PSEUDO=_recovery
RECOVERY=$USER RECOVERY=$USER
new "cred: none realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
testrun none $REALUSER $PSEUDO $RECOVERY true false testrun $CRED $REALUSER $PSEUDO $RECOVERY false false
for c in exact except; do
new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
testrun $c $REALUSER $PSEUDO $RECOVERY false false
done
#------- REALUSER: ROOT #------- CRED: except USER: root
#XXX: seems not to work in docker CRED=except
# Neither of these should work: user != recovery REALUSER=root
REALUSER=root
# Recovery as a seperate user does not work
PSEUDO=root PSEUDO=root
RECOVERY=_recovery RECOVERY=_recovery
for c in none exact except; do new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" testrun $CRED $REALUSER $PSEUDO $RECOVERY true false
testrun $c $REALUSER $PSEUDO $RECOVERY true false
done
# All these should work: user == recovery # Recovery as actual user works
REALUSER=root
PSEUDO=root PSEUDO=root
RECOVERY=root RECOVERY=root
for c in none exact except; do new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" testrun $CRED $REALUSER $PSEUDO $RECOVERY true true
testrun $c $REALUSER $PSEUDO $RECOVERY true true
done
# none and except credentials should work # pseudo-user as recovery user works IF cred=except AND realuser=root!
# XXX: except does not work in travis PSEUDO=_recovery
RECOVERY=_recovery
new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
testrun $CRED $REALUSER $PSEUDO $RECOVERY true true
PSEUDO=_recovery
RECOVERY=root
new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
testrun $CRED $REALUSER $PSEUDO $RECOVERY true false
#------- CRED: none
# Check you can use any pseudo user if cred is none
CRED=none
REALUSER=$USER
PSEUDO=_recovery
RECOVERY=_recovery
new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
testrun $CRED $REALUSER $PSEUDO $RECOVERY true true
#------- CRED: exact
# pseudo-user as recovery user does not work if cred=exact
CRED=exact
REALUSER=root REALUSER=root
PSEUDO=_recovery PSEUDO=_recovery
RECOVERY=_recovery RECOVERY=_recovery
for c in none except; do new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" testrun $CRED $REALUSER $PSEUDO $RECOVERY false false
testrun $c $REALUSER $PSEUDO $RECOVERY true true
done
new "cred: exact realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
testrun exact $REALUSER $PSEUDO $RECOVERY false false
# None of these work
REALUSER=root
PSEUDO=_recovery
RECOVERY=root
for c in none except; do
new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
testrun $c $REALUSER $PSEUDO $RECOVERY true false
done
new "cred: exact realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY"
testrun exact $REALUSER $PSEUDO $RECOVERY false false
rm -rf $dir rm -rf $dir

4
yang/clixon/clixon-config@2020-06-17.yang
View file

@ -47,7 +47,8 @@ module clixon-config {
"Added: CLICON_CLI_LINES_DEFAULT "Added: CLICON_CLI_LINES_DEFAULT
Added enum HIDE to CLICON_CLI_GENMODEL Added enum HIDE to CLICON_CLI_GENMODEL
Added CLICON_SSL_SERVER_CERT, CLICON_SSL_SERVER_KEY, CLICON_SSL_CA_CERT Added CLICON_SSL_SERVER_CERT, CLICON_SSL_SERVER_KEY, CLICON_SSL_CA_CERT
Added CLICON_NACM_DISABLED_ON_EMPTY"; Added CLICON_NACM_DISABLED_ON_EMPTY
Removed default valude of CLICON_NACM_RECOVERY_USER";
} }
revision 2020-04-23 { revision 2020-04-23 {
description description
@ -708,7 +709,6 @@ module clixon-config {
} }
leaf CLICON_NACM_RECOVERY_USER { leaf CLICON_NACM_RECOVERY_USER {
type string; type string;
default "_nacm_recovery";
description description
"RFC8341 defines a 'recovery session' as outside the scope. Clixon "RFC8341 defines a 'recovery session' as outside the scope. Clixon
defines this user as having special admin rights to exempt from defines this user as having special admin rights to exempt from