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

Fixed: [Netconf commit confirm session-id mismatch #407](https://github.com/clicon/clixon/issues/407)

Browse source
This commit is contained in:
Olof hagsand 2023-01-18 15:55:49 +01:00
parent 923b998774
commit 46137e1394
7 changed files with 36 additions and 28 deletions
Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -93,6 +93,7 @@ Developers may need to change their code
### Corrected Bugs ### Corrected Bugs
* Fixed: [Netconf commit confirm session-id mismatch #407](https://github.com/clicon/clixon/issues/407)
* Fixed: Initialized session-id to 1 instead of 0 following ietf-netconf.yang * Fixed: Initialized session-id to 1 instead of 0 following ietf-netconf.yang
* Fixed: [snmpwalk doesn't show properly SNMP boolean values which equal false](https://github.com/clicon/clixon/issues/400) * Fixed: [snmpwalk doesn't show properly SNMP boolean values which equal false](https://github.com/clicon/clixon/issues/400)
* Fixed: yang-library: Remove revision if empty instead of sending empty revision * Fixed: yang-library: Remove revision if empty instead of sending empty revision

2
apps/backend/backend_client.c
View file

@ -567,7 +567,7 @@ from_client_edit_config(clicon_handle h,
break; break;
} }
} }
if ((ret = candidate_commit(h, NULL, "candidate", cbret)) < 0){ /* Assume validation fail, nofatal */ if ((ret = candidate_commit(h, NULL, "candidate", myid, cbret)) < 0){ /* Assume validation fail, nofatal */
if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0) if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0)
goto done; goto done;
xmldb_copy(h, "running", "candidate"); xmldb_copy(h, "running", "candidate");

8
apps/backend/backend_commit.c
View file

@ -648,6 +648,7 @@ candidate_validate(clicon_handle h,
* @param[in] h Clicon handle * @param[in] h Clicon handle
* @param[in] xe Request: <rpc><xn></rpc> (or NULL) * @param[in] xe Request: <rpc><xn></rpc> (or NULL)
* @param[in] db A candidate database, not necessarily "candidate" * @param[in] db A candidate database, not necessarily "candidate"
* @param[in] myid Client id of triggering incoming message (or 0)
* @param[out] cbret Return xml tree, eg <rpc-reply>..., <rpc-error.. (if retval = 0) * @param[out] cbret Return xml tree, eg <rpc-reply>..., <rpc-error.. (if retval = 0)
* @retval 1 Validation OK * @retval 1 Validation OK
* @retval 0 Validation failed (with cbret set) * @retval 0 Validation failed (with cbret set)
@ -657,6 +658,7 @@ int
candidate_commit(clicon_handle h, candidate_commit(clicon_handle h,
cxobj *xe, cxobj *xe,
char *db, char *db,
uint32_t myid,
cbuf *cbret) cbuf *cbret)
{ {
int retval = -1; int retval = -1;
@ -693,7 +695,7 @@ candidate_commit(clicon_handle h,
if (if_feature(yspec, "ietf-netconf", "confirmed-commit") if (if_feature(yspec, "ietf-netconf", "confirmed-commit")
&& confirmed_commit_state_get(h) != ROLLBACK && confirmed_commit_state_get(h) != ROLLBACK
&& xe != NULL){ && xe != NULL){
if (handle_confirmed_commit(h, xe) < 0) if (handle_confirmed_commit(h, xe, myid) < 0)
goto done; goto done;
} }
if (ret == 0){ if (ret == 0){
@ -817,7 +819,7 @@ from_client_commit(clicon_handle h,
goto done; goto done;
goto ok; goto ok;
} }
if ((ret = candidate_commit(h, xe, "candidate", cbret)) < 0){ /* Assume validation fail, nofatal */ if ((ret = candidate_commit(h, xe, "candidate", myid, cbret)) < 0){ /* Assume validation fail, nofatal */
clicon_debug(1, "Commit candidate failed"); clicon_debug(1, "Commit candidate failed");
if (ret < 0) if (ret < 0)
if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0) if (netconf_operation_failed(cbret, "application", clicon_err_reason)< 0)
@ -1084,7 +1086,7 @@ load_failsafe(clicon_handle h,
goto done; goto done;
if (xmldb_db_reset(h, "running") < 0) if (xmldb_db_reset(h, "running") < 0)
goto done; goto done;
ret = candidate_commit(h, NULL, db, cbret); ret = candidate_commit(h, NULL, db, 0, cbret);
if (ret != 1) if (ret != 1)
if (xmldb_copy(h, "tmp", "running") < 0) if (xmldb_copy(h, "tmp", "running") < 0)
goto done; goto done;

43
apps/backend/backend_confirm.c
View file

@ -401,7 +401,7 @@ cancel_confirmed_commit(clicon_handle h)
* 2) be presented with a <persist-id> value that matches the <persist> value accompanying the prior confirmed-commit * 2) be presented with a <persist-id> value that matches the <persist> value accompanying the prior confirmed-commit
* *
* @param[in] h Clicon handle * @param[in] h Clicon handle
* @param[in] xe Request: <rpc><xn></rpc> * @param[in] xe Request: <rpc><xn></rpc>
* @param[in] myid current client session-id * @param[in] myid current client session-id
* @retval 1 The confirming-commit is valid * @retval 1 The confirming-commit is valid
* @retval 0 The confirming-commit is not valid * @retval 0 The confirming-commit is not valid
@ -419,6 +419,8 @@ check_valid_confirming_commit(clicon_handle h,
clicon_err(OE_CFG, EINVAL, "xe is NULL"); clicon_err(OE_CFG, EINVAL, "xe is NULL");
goto done; goto done;
} }
if (myid == 0)
goto invalid;
switch (confirmed_commit_state_get(h)) { switch (confirmed_commit_state_get(h)) {
case PERSISTENT: case PERSISTENT:
if (xe_persist_id(xe, &persist_id)) { if (xe_persist_id(xe, &persist_id)) {
@ -470,15 +472,18 @@ check_valid_confirming_commit(clicon_handle h,
* *
* @param[in] h Clicon handle * @param[in] h Clicon handle
* @param[in] xe Commit rpc xml or NULL * @param[in] xe Commit rpc xml or NULL
* @param[in] myid Current session-id, only valid > 0 if call is made as a result of an incoming message
* @retval 0 OK * @retval 0 OK
* @retval -1 Error * @retval -1 Error
* @note There are some calls to this function where myid is 0 (which is invalid). It is unclear if such calls
* actually occur, and if so, if they are correctly handled. The calls are from do_rollback() and load_failsafe()
*/ */
int int
handle_confirmed_commit(clicon_handle h, handle_confirmed_commit(clicon_handle h,
cxobj *xe) cxobj *xe,
uint32_t myid)
{ {
int retval = -1; int retval = -1;
uint32_t session_id;
char *persist; char *persist;
unsigned long confirm_timeout = 0L; unsigned long confirm_timeout = 0L;
int cc_valid; int cc_valid;
@ -488,17 +493,14 @@ handle_confirmed_commit(clicon_handle h,
clicon_err(OE_CFG, EINVAL, "xe is NULL"); clicon_err(OE_CFG, EINVAL, "xe is NULL");
goto done; goto done;
} }
if (clicon_session_id_get(h, &session_id) < 0) { if (myid == 0)
clicon_err(OE_DAEMON, 0, goto ok;
"an ephemeral confirmed-commit was issued, but the session-id could not be determined");
goto done;
};
/* The case of a valid confirming-commit is also handled in the first phase, but only if there is no subsequent /* The case of a valid confirming-commit is also handled in the first phase, but only if there is no subsequent
* confirmed-commit. It is tested again here as the case of a valid confirming-commit *with* a subsequent * confirmed-commit. It is tested again here as the case of a valid confirming-commit *with* a subsequent
* confirmed-commit must be handled once the transaction has begun and after all the plugins' validate callbacks * confirmed-commit must be handled once the transaction has begun and after all the plugins' validate callbacks
* have been called. * have been called.
*/ */
cc_valid = check_valid_confirming_commit(h, xe, session_id); cc_valid = check_valid_confirming_commit(h, xe, myid);
if (cc_valid) { if (cc_valid) {
if (cancel_rollback_event(h) < 0) { if (cancel_rollback_event(h) < 0) {
clicon_err(OE_DAEMON, 0, "A valid confirming-commit was received, but the corresponding rollback event was not found"); clicon_err(OE_DAEMON, 0, "A valid confirming-commit was received, but the corresponding rollback event was not found");
@ -538,7 +540,7 @@ handle_confirmed_commit(clicon_handle h,
/* The client did not pass a value for <persist> and therefore any subsequent confirming-commit must be /* The client did not pass a value for <persist> and therefore any subsequent confirming-commit must be
* issued within the same session. * issued within the same session.
*/ */
confirmed_commit_session_id_set(h, session_id); confirmed_commit_session_id_set(h, myid);
confirmed_commit_state_set(h, EPHEMERAL); confirmed_commit_state_set(h, EPHEMERAL);
clicon_log(LOG_INFO, clicon_log(LOG_INFO,
@ -603,6 +605,7 @@ handle_confirmed_commit(clicon_handle h,
goto done; goto done;
} }
} }
ok:
retval = 0; retval = 0;
done: done:
return retval; return retval;
@ -650,7 +653,7 @@ do_rollback(clicon_handle h,
confirmed_commit_persist_id_set(h, NULL); confirmed_commit_persist_id_set(h, NULL);
} }
confirmed_commit_state_set(h, ROLLBACK); confirmed_commit_state_set(h, ROLLBACK);
if (candidate_commit(h, NULL, "rollback", cbret) < 0) { /* Assume validation fail, nofatal */ if (candidate_commit(h, NULL, "rollback", 0, cbret) < 0) { /* Assume validation fail, nofatal */
/* theoretically, this should never error, since the rollback database was previously active and therefore /* theoretically, this should never error, since the rollback database was previously active and therefore
* had itself been previously and successfully committed. * had itself been previously and successfully committed.
*/ */
@ -713,12 +716,14 @@ from_client_cancel_commit(clicon_handle h,
void *arg, void *arg,
void *regarg) void *regarg)
{ {
cxobj *persist_id_xml; cxobj *persist_id_xml;
char *persist_id = NULL; char *persist_id = NULL;
uint32_t cur_session_id; uint32_t myid;
int retval = -1; int retval = -1;
int rollback = 0; int rollback = 0;
struct client_entry *ce = (struct client_entry *)arg;
myid = ce->ce_id;
if ((persist_id_xml = xml_find_type(xe, NULL, "persist-id", CX_ELMNT)) != NULL) { if ((persist_id_xml = xml_find_type(xe, NULL, "persist-id", CX_ELMNT)) != NULL) {
/* persist == persist_id == NULL is legal */ /* persist == persist_id == NULL is legal */
persist_id = xml_body(persist_id_xml); persist_id = xml_body(persist_id_xml);
@ -729,11 +734,7 @@ from_client_cancel_commit(clicon_handle h,
if (netconf_invalid_value(cbret, "protocol", "current confirmed-commit is not persistent") < 0) if (netconf_invalid_value(cbret, "protocol", "current confirmed-commit is not persistent") < 0)
goto done; goto done;
} }
else if (clicon_session_id_get(h, &cur_session_id) < 0) { else if (myid != confirmed_commit_session_id_get(h)) {
if (netconf_invalid_value(cbret, "application", "session-id was not set") < 0)
goto done;
}
else if (cur_session_id != confirmed_commit_session_id_get(h)) {
if (netconf_invalid_value(cbret, "protocol", "confirming-commit must be given within session that gave the confirmed-commit") < 0) if (netconf_invalid_value(cbret, "protocol", "confirming-commit must be given within session that gave the confirmed-commit") < 0)
goto done; goto done;
} }

4
apps/backend/clixon_backend_commit.h
View file

@ -62,7 +62,7 @@ enum confirmed_commit_state confirmed_commit_state_get(clicon_handle h);
uint32_t confirmed_commit_session_id_get(clicon_handle h); uint32_t confirmed_commit_session_id_get(clicon_handle h);
int cancel_rollback_event(clicon_handle h); int cancel_rollback_event(clicon_handle h);
int cancel_confirmed_commit(clicon_handle h); int cancel_confirmed_commit(clicon_handle h);
int handle_confirmed_commit(clicon_handle h, cxobj *xe); int handle_confirmed_commit(clicon_handle h, cxobj *xe, uint32_t myid);
int do_rollback(clicon_handle h, uint8_t *errs); int do_rollback(clicon_handle h, uint8_t *errs);
int from_client_cancel_commit(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg); int from_client_cancel_commit(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg);
int from_client_confirmed_commit(clicon_handle h, cxobj *xe, uint32_t myid, cbuf *cbret); int from_client_confirmed_commit(clicon_handle h, cxobj *xe, uint32_t myid, cbuf *cbret);
@ -71,7 +71,7 @@ int from_client_confirmed_commit(clicon_handle h, cxobj *xe, uint32_t myid, cbuf
int startup_validate(clicon_handle h, char *db, cxobj **xtr, cbuf *cbret); int startup_validate(clicon_handle h, char *db, cxobj **xtr, cbuf *cbret);
int startup_commit(clicon_handle h, char *db, cbuf *cbret); int startup_commit(clicon_handle h, char *db, cbuf *cbret);
int candidate_validate(clicon_handle h, char *db, cbuf *cbret); int candidate_validate(clicon_handle h, char *db, cbuf *cbret);
int candidate_commit(clicon_handle h, cxobj *xe, char *db, cbuf *cbret); int candidate_commit(clicon_handle h, cxobj *xe, char *db, uint32_t myid, cbuf *cbret);
int from_client_commit(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg); int from_client_commit(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg);
int from_client_discard_changes(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg); int from_client_discard_changes(clicon_handle h, cxobj *xe, cbuf *cbret, void *arg, void *regarg);

4
lib/src/clixon_data.c
View file

@ -808,12 +808,16 @@ clicon_db_elmnt_set(clicon_handle h,
} }
/*! Get session id /*! Get session id
*
* @param[in] h Clicon handle * @param[in] h Clicon handle
* @param[out] sid Session identifier * @param[out] sid Session identifier
* @retval 0 OK * @retval 0 OK
* @retval -1 Session id not set * @retval -1 Session id not set
* Session-ids survive TCP sessions that are created for each message sent to the backend. * Session-ids survive TCP sessions that are created for each message sent to the backend.
* The backend assigns session-id for clients: backend assigns, clients get it from backend. * The backend assigns session-id for clients: backend assigns, clients get it from backend.
* @note a client will get the currrent session-id of that client, BUT
* a backend will get the next session-id to be assigned
* A backend getting a session-id of an ongoing session should use ce->ce_id
*/ */
int int
clicon_session_id_get(clicon_handle h, clicon_session_id_get(clicon_handle h,

2
util/clixon_util_validate.c
View file

@ -227,7 +227,7 @@ main(int argc,
goto done; goto done;
} }
if (commit){ if (commit){
if ((ret = candidate_commit(h, NULL, database, cb)) < 0) if ((ret = candidate_commit(h, NULL, database, 0, cb)) < 0)
goto done; goto done;
} }
else{ else{