From e3f3d772c70bfdd24b77af70cde29b48aa9bdbef Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Wed, 28 Sep 2022 15:04:11 +0200 Subject: [PATCH] Fixed: [message-id present on netconf app "hello"](https://github.com/clicon/clixon/issues/369) --- CHANGELOG.md | 1 + apps/backend/backend_client.c | 9 ++------- apps/netconf/netconf_main.c | 4 +++- lib/src/clixon_netconf_lib.c | 3 +-- lib/src/clixon_proto_client.c | 2 +- lib/src/clixon_xml_bind.c | 2 ++ lib/src/clixon_xml_io.c | 5 +++-- test/lib.sh | 6 +++--- test/test_netconf_hello.sh | 7 ++++--- test/test_netconf_ssh_callhome.sh | 2 +- test/test_sock.sh | 4 ++-- 11 files changed, 23 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14a3da29..d1dddc2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ Expected: November 2022 ### Corrected Bugs +* Fixed: [message-id present on netconf app "hello"](https://github.com/clicon/clixon/issues/369) * Fixed: [SNMP "smiv2" yang extension doesn't work on augmented nodes](https://github.com/clicon/clixon/issues/366) ## 5.9.0 diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index b5fdabca..7164bca7 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -1183,7 +1183,6 @@ from_client_hello(clicon_handle h, { int retval = -1; uint32_t id; - char *msgid; if (clicon_session_id_get(h, &id) < 0){ clicon_err(OE_NETCONF, ENOENT, "session_id not set"); @@ -1191,12 +1190,8 @@ from_client_hello(clicon_handle h, } id++; clicon_session_id_set(h, id); - if ((msgid = xml_find_value(x, "message-id")) != NULL) - cprintf(cbret, "%u", - NETCONF_BASE_NAMESPACE, msgid, id); - else - cprintf(cbret, "%u", - NETCONF_BASE_NAMESPACE, id); + cprintf(cbret, "%u", + NETCONF_BASE_NAMESPACE, id); retval = 0; done: return retval; diff --git a/apps/netconf/netconf_main.c b/apps/netconf/netconf_main.c index 35fbb598..b6208acb 100644 --- a/apps/netconf/netconf_main.c +++ b/apps/netconf/netconf_main.c @@ -434,7 +434,9 @@ netconf_input_frame(clicon_handle h, if (ret == 0){ /* Note: xtop can be "hello" in which case one (maybe) should drop the session and log * However, its not until netconf_input_packet that rpc vs hello vs other identification is - * actually made + * actually made. + * Actually, there are no error replies to hello messages according to any RFC, so + * rpc error reply here is non-standard, but may be useful. */ if ((cbret = cbuf_new()) == NULL){ clicon_err(OE_XML, errno, "cbuf_new"); diff --git a/lib/src/clixon_netconf_lib.c b/lib/src/clixon_netconf_lib.c index e8ff8836..6d7d795e 100644 --- a/lib/src/clixon_netconf_lib.c +++ b/lib/src/clixon_netconf_lib.c @@ -1767,8 +1767,7 @@ netconf_hello_server(clicon_handle h, char *encstr = NULL; module_set_id = clicon_option_str(h, "CLICON_MODULE_SET_ID"); - - cprintf(cb, "", NETCONF_BASE_NAMESPACE, 42); + cprintf(cb, "", NETCONF_BASE_NAMESPACE); cprintf(cb, ""); if (clicon_option_int(h, "CLICON_NETCONF_BASE_CAPABILITY") > 0){ /* Each peer MUST send at least the base NETCONF capability, "urn:ietf:params:netconf:base:1.1" diff --git a/lib/src/clixon_proto_client.c b/lib/src/clixon_proto_client.c index 63ce9785..0781eaa9 100644 --- a/lib/src/clixon_proto_client.c +++ b/lib/src/clixon_proto_client.c @@ -1506,7 +1506,7 @@ clicon_hello_req(clicon_handle h, int ret; username = clicon_username_get(h); - if ((msg = clicon_msg_encode(0, "%s", + if ((msg = clicon_msg_encode(0, "%s", username?username:"", NETCONF_BASE_NAMESPACE, NETCONF_BASE_CAPABILITY_1_1)) == NULL) diff --git a/lib/src/clixon_xml_bind.c b/lib/src/clixon_xml_bind.c index e6d88787..daac2cfd 100644 --- a/lib/src/clixon_xml_bind.c +++ b/lib/src/clixon_xml_bind.c @@ -642,6 +642,8 @@ xml_bind_yang_rpc(cxobj *xrpc, /* Hello: dont bind, dont appear in any yang spec, just ensure there is nothing apart from * session-id or capabilities/capability tags * If erro, just log, drop and close, rpc-error should not be sent since it is not rpc + * Actually, there are no error replies to hello messages according to any RFC, so + * rpc error reply here is non-standard, but may be useful. */ x = NULL; while ((x = xml_child_each(xrpc, x, CX_ELMNT)) != NULL) { diff --git a/lib/src/clixon_xml_io.c b/lib/src/clixon_xml_io.c index 5f9d49ea..3f469893 100644 --- a/lib/src/clixon_xml_io.c +++ b/lib/src/clixon_xml_io.c @@ -528,6 +528,7 @@ xmltree2cbuf(cbuf *cb, * therefore not well-formed. * Therefore checking for empty XML must be done by a calling function which knows wether the * the XML represents a full document or not. + * @note may be called recursively, some yang-bind (eg rpc) semantic checks may trigger error message */ static int _xml_parse(const char *str, @@ -855,8 +856,8 @@ clixon_xml_attr_copy(cxobj *xin, clicon_err(OE_XML, EINVAL, "xin or xout NULL"); goto done; } - if ((msgid = xml_find_value(xin, "message-id")) != NULL){ - if ((xa = xml_new("message-id", xout, CX_ATTR)) == NULL) + if ((msgid = xml_find_value(xin, name)) != NULL){ + if ((xa = xml_new(name, xout, CX_ATTR)) == NULL) goto done; if (xml_value_set(xa, msgid) < 0) goto done; diff --git a/test/lib.sh b/test/lib.sh index 73d4354a..ac39a21d 100755 --- a/test/lib.sh +++ b/test/lib.sh @@ -89,14 +89,14 @@ LIBNS='xmlns="http://clicon.org/lib"' # Namespace: Clixon restconf RESTCONFNS='xmlns="http://clicon.org/restconf"' -# Default netconf namespace statement, typically as placed on top-level urn:ietf:params:netconf:base:1.0urn:ietf:params:netconf:base:1.1]]>]]>" +DEFAULTHELLO="urn:ietf:params:netconf:base:1.0urn:ietf:params:netconf:base:1.1]]>]]>" # XXX cannot get this to work for all combinations of nc/netcat fcgi/native # But leave it here for debugging where netcat works properly diff --git a/test/test_netconf_hello.sh b/test/test_netconf_hello.sh index ad5133f7..93e81177 100755 --- a/test/test_netconf_hello.sh +++ b/test/test_netconf_hello.sh @@ -96,7 +96,7 @@ new "Netconf snd hello with base capability with extra arguments" expecteof "$clixon_netconf -qef $cfg" 0 "urn:ietf:params:netconf:base:1.1?arg=val]]>]]>" '^$' new "Netconf hello with wrong namespace -> terminate" -expecteof "$clixon_netconf -qef $cfg" 255 "urn:ietf:params:netconf:base:1.1]]>]]>" '^$' 2> /dev/null +expecteof "$clixon_netconf -qef $cfg" 255 "urn:ietf:params:netconf:base:1.1]]>]]>" '^$' 2> /dev/null # This is hello - shouldnt really get rpc back? new "Netconf snd hello with wrong prefix" @@ -106,10 +106,11 @@ new "Netconf snd hello with prefix" expecteof "$clixon_netconf -qef $cfg" 0 "urn:ietf:params:netconf:base:1.1]]>]]>" '^$' new "netconf snd + rcv hello" -expecteof "$clixon_netconf -f $cfg" 0 "urn:ietf:params:netconf:base:1.1]]>]]>" "^urn:ietf:params:netconf:base:1.1urn:ietf:params:netconf:base:1.0urn:ietf:params:netconf:capability:yang-library:1.0?revision=2019-01-04&module-set-id=42urn:ietf:params:netconf:capability:candidate:1.0urn:ietf:params:netconf:capability:validate:1.1urn:ietf:params:netconf:capability:startup:1.0urn:ietf:params:netconf:capability:xpath:1.0urn:ietf:params:netconf:capability:notification:1.0urn:ietf:params:netconf:capability:with-defaults:1.0?basic-mode=explicit&also-supported=report-all,trim,report-all-tagged[0-9]*]]>]]>$" '^$' +expecteof "$clixon_netconf -f $cfg" 0 "urn:ietf:params:netconf:base:1.1]]>]]>" "^urn:ietf:params:netconf:base:1.1urn:ietf:params:netconf:base:1.0urn:ietf:params:netconf:capability:yang-library:1.0?revision=2019-01-04&module-set-id=42urn:ietf:params:netconf:capability:candidate:1.0urn:ietf:params:netconf:capability:validate:1.1urn:ietf:params:netconf:capability:startup:1.0urn:ietf:params:netconf:capability:xpath:1.0urn:ietf:params:netconf:capability:notification:1.0urn:ietf:params:netconf:capability:with-defaults:1.0?basic-mode=explicit&also-supported=report-all,trim,report-all-tagged[0-9]*]]>]]>$" '^$' +# Actually non-standard to reply on wrong hello with rpc-error, but may be useful new "Netconf snd hello with extra element" -expecteof "$clixon_netconf -qef $cfg" 0 "urn:ietf:params:netconf:base:1.1]]>]]>" '^protocolunknown-elementextra-elementerrorUnrecognized hello/capabilities element]]>]]>$' '^$' +expecteof "$clixon_netconf -qef $cfg" 0 "urn:ietf:params:netconf:base:1.1]]>]]>" '^protocolunknown-elementextra-elementerrorUnrecognized hello/capabilities element]]>]]>$' '^$' new "Netconf send rpc without hello error" expecteof "$clixon_netconf -qef $cfg" 255 "]]>]]>" "rpcoperation-failederrorClient must send an hello element before any RPC]]>]]>" '^$' diff --git a/test/test_netconf_ssh_callhome.sh b/test/test_netconf_ssh_callhome.sh index f87132f1..32a05415 100755 --- a/test/test_netconf_ssh_callhome.sh +++ b/test/test_netconf_ssh_callhome.sh @@ -134,7 +134,7 @@ EOF new "Start Listener client" echo "ssh -s -F $sshcfg -v -i $key -o ProxyUseFdpass=yes -o ProxyCommand=\"clixon_netconf_ssh_callhome_client -a 127.0.0.1\" . netconf" #-F $sshcfg -expectpart "$(ssh -s -F $sshcfg -v -i $key -o ProxyUseFdpass=yes -o ProxyCommand="${clixon_netconf_ssh_callhome_client} -a 127.0.0.1" . netconf < $rpccmd)" 0 "urn:ietf:params:netconf:base:1.1urn:ietf:params:netconf:base:1.0urn:ietf:params:netconf:capability:yang-library:1.0?revision=2019-01-04&module-set-id=42urn:ietf:params:netconf:capability:candidate:1.0urn:ietf:params:netconf:capability:validate:1.1urn:ietf:params:netconf:capability:startup:1.0urn:ietf:params:netconf:capability:xpath:1.0urn:ietf:params:netconf:capability:notification:1.0urn:ietf:params:netconf:capability:with-defaults:1.0?basic-mode=explicit&also-supported=report-all,trim,report-all-tagged2" "" +expectpart "$(ssh -s -F $sshcfg -v -i $key -o ProxyUseFdpass=yes -o ProxyCommand="${clixon_netconf_ssh_callhome_client} -a 127.0.0.1" . netconf < $rpccmd)" 0 "urn:ietf:params:netconf:base:1.1urn:ietf:params:netconf:base:1.0urn:ietf:params:netconf:capability:yang-library:1.0?revision=2019-01-04&module-set-id=42urn:ietf:params:netconf:capability:candidate:1.0urn:ietf:params:netconf:capability:validate:1.1urn:ietf:params:netconf:capability:startup:1.0urn:ietf:params:netconf:capability:xpath:1.0urn:ietf:params:netconf:capability:notification:1.0urn:ietf:params:netconf:capability:with-defaults:1.0?basic-mode=explicit&also-supported=report-all,trim,report-all-tagged2" "" # Wait wait diff --git a/test/test_sock.sh b/test/test_sock.sh index fb1bdd74..0a604c3b 100755 --- a/test/test_sock.sh +++ b/test/test_sock.sh @@ -72,10 +72,10 @@ EOF expectpart "$($clixon_cli -1f $cfg show version)" 0 "${CLIXON_VERSION}" new "hello session-id 2" - expecteof "$clixon_util_socket -a $family -s $sock -D $DBG" 0 "" "3" + expecteof "$clixon_util_socket -a $family -s $sock -D $DBG" 0 "" "3" new "hello session-id 2" - expecteof "$clixon_util_socket -a $family -s $sock -D $DBG" 0 "" "4" + expecteof "$clixon_util_socket -a $family -s $sock -D $DBG" 0 "" "4" if [ $BE -ne 0 ]; then new "Kill backend"