From dc1ad560f9f9ff608333f8efffe9715e56c8c3b5 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Tue, 11 Aug 2020 14:46:53 +0200 Subject: [PATCH] * 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. * 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 --- CHANGELOG.md | 17 +- apps/backend/backend_client.c | 41 ++++- apps/cli/cli_common.c | 4 - apps/restconf/restconf_lib.c | 3 +- apps/restconf/restconf_methods.c | 183 ++++++---------------- apps/restconf/restconf_methods_post.c | 56 ++----- lib/src/clixon_data.c | 2 +- lib/src/clixon_datastore_write.c | 49 +++++- lib/src/clixon_netconf_lib.c | 2 +- test/test_nacm_protocol.sh | 2 +- test/test_nacm_recovery.sh | 115 ++++++-------- yang/clixon/clixon-config@2020-06-17.yang | 4 +- 12 files changed, 217 insertions(+), 261 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 668ad8c9..43268dda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. * 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). + * 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) * This applies if NACM is loaded and `CLICON_NACM_MODE` is `internal` * 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 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. +* 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 * 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". @@ -66,6 +72,7 @@ Expected: July 2020 * CLICON_SSL_SERVER_KEY * CLICON_SSL_CA_CERT * 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): * Bodies in error retuns including html code have been removed * Some (extra) CRLF:s have been removed @@ -100,6 +107,15 @@ Expected: July 2020 ### 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. * 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. @@ -112,7 +128,6 @@ Expected: July 2020 ### 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). * 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' diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index e3be78a5..e1d94ef6 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -576,6 +576,9 @@ from_client_edit_config(clicon_handle h, int ret; char *username; cxobj *xret = NULL; + char *attr; + int autocommit = 0; + char *val = NULL; username = clicon_username_get(h); if ((yspec = clicon_dbspec_yang(h)) == NULL){ @@ -605,7 +608,6 @@ from_client_edit_config(clicon_handle h, goto done; goto ok; } - if ((x = xpath_first(xn, NULL, "default-operation")) != NULL){ if (xml_operation(xml_body(x), &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) goto ok; 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); - cprintf(cbret, ""); + cprintf(cbret, ""); ok: retval = 0; done: @@ -1026,7 +1061,7 @@ from_client_get(clicon_handle h, content = netconf_content_str2int(attr); /* Clixon extensions: depth */ if ((attr = xml_find_value(xe, "depth")) != NULL){ - char *reason = NULL; + char *reason = NULL; if ((ret = parse_int32(attr, &depth, &reason)) < 0){ clicon_err(OE_XML, errno, "parse_int32"); goto done; diff --git a/apps/cli/cli_common.c b/apps/cli/cli_common.c index 1e1e8f00..89755ff9 100644 --- a/apps/cli/cli_common.c +++ b/apps/cli/cli_common.c @@ -312,10 +312,6 @@ cli_dbxml(clicon_handle h, goto done; if (clicon_rpc_edit_config(h, "candidate", OP_NONE, cbuf_get(cb)) < 0) goto done; - if (clicon_autocommit(h)) { - if (clicon_rpc_commit(h) < 0) - goto done; - } retval = 0; done: if (xerr) diff --git a/apps/restconf/restconf_lib.c b/apps/restconf/restconf_lib.c index ea8b2cfe..73e8ebf9 100644 --- a/apps/restconf/restconf_lib.c +++ b/apps/restconf/restconf_lib.c @@ -258,7 +258,8 @@ restconf_terminate(clicon_handle h) 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] qvec Query parameters (eg where insert/point should be) * @retval 0 OK diff --git a/apps/restconf/restconf_methods.c b/apps/restconf/restconf_methods.c index 4a299a91..c1389940 100644 --- a/apps/restconf/restconf_methods.c +++ b/apps/restconf/restconf_methods.c @@ -272,7 +272,8 @@ api_data_write(clicon_handle h, cvec *nsc = NULL; yang_bind yb; char *xpath = NULL; - + char *attr; + clicon_debug(1, "%s api_path:\"%s\"", __FUNCTION__, api_path0); clicon_debug(1, "%s data:\"%s\"", __FUNCTION__, data); if ((yspec = clicon_dbspec_yang(h)) == NULL){ @@ -297,43 +298,10 @@ api_data_write(clicon_handle h, goto ok; } } - - xret = NULL; - if (clicon_rpc_get_config(h, clicon_nacm_recovery_user(h), - "candidate", xpath, nsc, &xret) < 0){ - 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; - } + if (plain_patch) + op = OP_MERGE; /* bad request if it does not exist */ + else + op = OP_REPLACE; /* OP_CREATE if it does not exist */ /* Create config top-of-tree */ if ((xtop = xml_new("config", NULL, CX_ELMNT)) == NULL) goto done; @@ -491,9 +459,20 @@ api_data_write(clicon_handle h, goto done; if (xml_prefix_set(xa, NETCONF_BASE_PREFIX) < 0) 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; - + 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 * Replace xparent with x, ie bottom of api-path with data */ @@ -608,7 +587,17 @@ api_data_write(clicon_handle h, username?username:"", NETCONF_BASE_PREFIX, NETCONF_BASE_NAMESPACE); /* bind nc to netconf namespace */ - cprintf(cbx, ""); + cprintf(cbx, ""); cprintf(cbx, "none"); if (clicon_xml2cbuf(cbx, xtop, 0, 0, -1) < 0) goto done; @@ -621,58 +610,15 @@ api_data_write(clicon_handle h, goto done; goto ok; } - 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, "", clicon_nacm_recovery_user(h)); - cprintf(cbx, ""); - 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, "", username?username:""); - cprintf(cbx, ""); - if (clicon_rpc_netconf(h, cbuf_get(cbx), &xretdis, NULL) < 0) + if ((xe = xpath_first(xret, NULL, "//ok")) != NULL && + (attr = xml_find_value(xe, "objectexisted")) != NULL && + strcmp(attr, "false")==0){ + if (restconf_reply_send(req, 201, NULL) < 0) /* Created */ 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, "", clicon_nacm_recovery_user(h)); - cprintf(cbx, ""); - 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) + else + if (restconf_reply_send(req, 204, NULL) < 0) /* No content */ goto done; - } - else{ - if (restconf_reply_send(req, 204, NULL) < 0) - goto done; - } ok: retval = 0; done: @@ -877,7 +823,18 @@ api_data_delete(clicon_handle h, username?username:"", NETCONF_BASE_PREFIX, NETCONF_BASE_NAMESPACE); /* bind nc to netconf namespace */ - cprintf(cbx, ""); + + cprintf(cbx, ""); cprintf(cbx, "none"); if (clicon_xml2cbuf(cbx, xtop, 0, 0, -1) < 0) goto done; @@ -889,50 +846,6 @@ api_data_delete(clicon_handle h, goto done; 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, "", clicon_nacm_recovery_user(h)); - cprintf(cbx, ""); - 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, "", clicon_nacm_recovery_user(h)); - cprintf(cbx, ""); - 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, "", clicon_nacm_recovery_user(h)); - cprintf(cbx, ""); - 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) goto done; ok: diff --git a/apps/restconf/restconf_methods_post.c b/apps/restconf/restconf_methods_post.c index 2f166b3c..735dd6e7 100644 --- a/apps/restconf/restconf_methods_post.c +++ b/apps/restconf/restconf_methods_post.c @@ -370,7 +370,18 @@ api_data_post(clicon_handle h, username?username:"", NETCONF_BASE_PREFIX, NETCONF_BASE_NAMESPACE); /* bind nc to netconf namespace */ - cprintf(cbx, ""); + + cprintf(cbx, ""); cprintf(cbx, "none"); if (clicon_xml2cbuf(cbx, xtop, 0, 0, -1) < 0) goto done; @@ -383,49 +394,6 @@ api_data_post(clicon_handle h, goto done; 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, "", clicon_nacm_recovery_user(h)); - cprintf(cbx, ""); - 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, "", username?username:""); - cprintf(cbx, ""); - 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, "", clicon_nacm_recovery_user(h)); - cprintf(cbx, ""); - 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) goto done; if (restconf_reply_send(req, 201, NULL) < 0) diff --git a/lib/src/clixon_data.c b/lib/src/clixon_data.c index e46b2b6e..94b9cb22 100644 --- a/lib/src/clixon_data.c +++ b/lib/src/clixon_data.c @@ -83,7 +83,7 @@ * @param[in] name Data name * @param[out] val Data value as string * @retval 0 OK - * @retval -1 Not found + * @retval -1 Not found (or error) * @see clicon_option_str */ int diff --git a/lib/src/clixon_datastore_write.c b/lib/src/clixon_datastore_write.c index ba1e45aa..532c4720 100644 --- a/lib/src/clixon_datastore_write.c +++ b/lib/src/clixon_datastore_write.c @@ -119,7 +119,8 @@ attr_ns_value(cxobj *x, goto fail; } /* the attribute exists, but not w expected namespace */ - if (strcmp(ans, ns) == 0) + if (ns == NULL || + strcmp(ans, ns) == 0) val = xml_value(xa); } *valp = val; @@ -257,10 +258,14 @@ text_modify(clicon_handle h, enum insert_type insert = INS_LAST; int changed = 0; /* Only if x0p's children have changed-> sort necessary */ 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 */ - if ((ret = attr_ns_value(x1, - "operation", NETCONF_BASE_NAMESPACE, + if ((ret = attr_ns_value(x1, "operation", NETCONF_BASE_NAMESPACE, cbret, &opstr)) < 0) goto done; if (ret == 0) @@ -268,6 +273,28 @@ text_modify(clicon_handle h, if (opstr != NULL) if (xml_operation(opstr, &op) < 0) 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); if (yang_keyword_get(y0) == Y_LEAF_LIST || yang_keyword_get(y0) == Y_LEAF){ @@ -708,7 +735,8 @@ text_modify_top(clicon_handle h, yang_stmt *ymod;/* yang module */ char *opstr; int ret; - + char *createstr = NULL; + /* Check for operations embedded in tree according to netconf */ if ((ret = attr_ns_value(x1, "operation", NETCONF_BASE_NAMESPACE, @@ -719,7 +747,11 @@ text_modify_top(clicon_handle h, if (opstr != NULL) if (xml_operation(opstr, &op) < 0) goto done; - /* Special case if x1 is empty, top-level only */ + 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 */ if (xml_child_nr_type(x1, CX_ELMNT) == 0){ if (xml_child_nr_type(x0, CX_ELMNT)){ /* base tree not empty */ switch(op){ @@ -760,6 +792,12 @@ text_modify_top(clicon_handle h, } /* Special case top-level replace */ 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 ((ret = nacm_datanode_write(h, x1, x1t, NACM_UPDATE, username, xnacm, cbret)) < 0) goto done; @@ -951,6 +989,7 @@ xmldb_put(clicon_handle h, permit = (xnacm==NULL); /* 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 * new tree is made. diff --git a/lib/src/clixon_netconf_lib.c b/lib/src/clixon_netconf_lib.c index 266fa62b..c4cbb850 100644 --- a/lib/src/clixon_netconf_lib.c +++ b/lib/src/clixon_netconf_lib.c @@ -838,7 +838,7 @@ netconf_data_exists(cbuf *cb, * does not exist. For example, a "delete" operation was attempted on * data that does not exist. * @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 */ int diff --git a/test/test_nacm_protocol.sh b/test/test_nacm_protocol.sh index bc072b75..bb001b67 100755 --- a/test/test_nacm_protocol.sh +++ b/test/test_nacm_protocol.sh @@ -210,7 +210,7 @@ new "commit it" expecteof "$clixon_netconf -qf $cfg" 0 "]]>]]>" "^]]>]]>$" 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 new "permit-edit-config: limited ok restconf" diff --git a/test/test_nacm_recovery.sh b/test/test_nacm_recovery.sh index 2b661e01..834bcd42 100755 --- a/test/test_nacm_recovery.sh +++ b/test/test_nacm_recovery.sh @@ -4,7 +4,7 @@ # the config even though NACM is enabled and write is DENY # Only use netconf - restconf also has authentication on web level, and that gets # 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) @@ -148,89 +148,78 @@ EOF fi } -#------- REALUSER: $USER - -# Neither of these should work: user != recovery +#------- CRED: except USER: non-root +# This is default, therefore first +CRED=except REALUSER=$USER + +# Recovery as a seperate user does not work PSEUDO=$USER RECOVERY=_recovery -for c in none exact except; do - new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" - testrun $c $REALUSER $PSEUDO $RECOVERY true false -done +new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" +testrun $CRED $REALUSER $PSEUDO $RECOVERY true false -# All these should work: user == recovery -REALUSER=$USER +# Recovery as actual user works PSEUDO=$USER RECOVERY=$USER -for c in none exact except; do - new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" - testrun $c $REALUSER $PSEUDO $RECOVERY true true -done +new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" +testrun $CRED $REALUSER $PSEUDO $RECOVERY true true -# Only none credentials should work -REALUSER=$USER +# pseudo-user as recovery user does not work if actual user is non-root/non-web PSEUDO=_recovery RECOVERY=_recovery -new "cred: none realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" -testrun none $REALUSER $PSEUDO $RECOVERY true true -for c in exact except; do - new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" - testrun $c $REALUSER $PSEUDO $RECOVERY false false -done +new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" +testrun $CRED $REALUSER $PSEUDO $RECOVERY false false -# None of these work -REALUSER=$USER PSEUDO=_recovery RECOVERY=$USER -new "cred: none realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" -testrun none $REALUSER $PSEUDO $RECOVERY true false -for c in exact except; do - new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" - testrun $c $REALUSER $PSEUDO $RECOVERY false false -done +new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" +testrun $CRED $REALUSER $PSEUDO $RECOVERY false false -#------- REALUSER: ROOT -#XXX: seems not to work in docker -# Neither of these should work: user != recovery -REALUSER=root +#------- CRED: except USER: root +CRED=except +REALUSER=root + +# Recovery as a seperate user does not work PSEUDO=root RECOVERY=_recovery -for c in none exact except; do - new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" - testrun $c $REALUSER $PSEUDO $RECOVERY true false -done +new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" +testrun $CRED $REALUSER $PSEUDO $RECOVERY true false -# All these should work: user == recovery -REALUSER=root +# Recovery as actual user works PSEUDO=root RECOVERY=root -for c in none exact except; do - new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" - testrun $c $REALUSER $PSEUDO $RECOVERY true true -done +new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" +testrun $CRED $REALUSER $PSEUDO $RECOVERY true true -# none and except credentials should work -# XXX: except does not work in travis +# pseudo-user as recovery user works IF cred=except AND realuser=root! +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 PSEUDO=_recovery RECOVERY=_recovery -for c in none except; do - new "cred: $c realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" - 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 +new "cred: $CRED realuser:$REALUSER pseudo:$PSEUDO recovery:$RECOVERY" +testrun $CRED $REALUSER $PSEUDO $RECOVERY false false rm -rf $dir diff --git a/yang/clixon/clixon-config@2020-06-17.yang b/yang/clixon/clixon-config@2020-06-17.yang index 2a3c7de0..a87c758d 100644 --- a/yang/clixon/clixon-config@2020-06-17.yang +++ b/yang/clixon/clixon-config@2020-06-17.yang @@ -47,7 +47,8 @@ module clixon-config { "Added: CLICON_CLI_LINES_DEFAULT Added enum HIDE to CLICON_CLI_GENMODEL 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 { description @@ -708,7 +709,6 @@ module clixon-config { } leaf CLICON_NACM_RECOVERY_USER { type string; - default "_nacm_recovery"; description "RFC8341 defines a 'recovery session' as outside the scope. Clixon defines this user as having special admin rights to exempt from