Fixed: [message-id present on netconf app "hello"](#369)

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

apps/backend/backend_client.c

@@ -1183,20 +1183,15 @@ 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");
 	goto done;
     }
     id++;
     clicon_session_id_set(h, id);
-    if ((msgid = xml_find_value(x, "message-id")) != NULL)
-	cprintf(cbret, "<hello xmlns=\"%s\" message-id=\"%s\"><session-id>%u</session-id></hello>",
-		NETCONF_BASE_NAMESPACE, msgid, id);
-    else
-	cprintf(cbret, "<hello xmlns=\"%s\"><session-id>%u</session-id></hello>",
-		NETCONF_BASE_NAMESPACE, id);
+    cprintf(cbret, "<hello xmlns=\"%s\"><session-id>%u</session-id></hello>",
+	    NETCONF_BASE_NAMESPACE, id);
     retval = 0;
  done:
     return retval;

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");

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, "<hello xmlns=\"%s\" message-id=\"%u\">", NETCONF_BASE_NAMESPACE, 42);
+    cprintf(cb, "<hello xmlns=\"%s\">", NETCONF_BASE_NAMESPACE);
     cprintf(cb, "<capabilities>");
     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" 

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, "<hello username=\"%s\" xmlns=\"%s\" message-id=\"42\"><capabilities><capability>%s</capability></capabilities></hello>",
+    if ((msg = clicon_msg_encode(0, "<hello username=\"%s\" xmlns=\"%s\"><capabilities><capability>%s</capability></capabilities></hello>",
 				 username?username:"",
 				 NETCONF_BASE_NAMESPACE,
 				 NETCONF_BASE_CAPABILITY_1_1)) == NULL)

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) {

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;

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 <rpc xmlns=""
+# Default netconf namespace statement, typically as placed on top-level <hello xmlns=""
 DEFAULTONLY="xmlns=\"$BASENS\""
 
-# Default netconf namespace + message-id
+# Default netconf namespace + message-id, ie for <rpc xmlns="" message-id="", but NOT for hello
 DEFAULTNS="$DEFAULTONLY message-id=\"42\""
 
 # Minimal hello message as a prelude to netconf rpcs
-DEFAULTHELLO="<?xml version=\"1.0\" encoding=\"UTF-8\"?><hello $DEFAULTNS><capabilities><capability>urn:ietf:params:netconf:base:1.0</capability><capability>urn:ietf:params:netconf:base:1.1</capability></capabilities></hello>]]>]]>"
+DEFAULTHELLO="<?xml version=\"1.0\" encoding=\"UTF-8\"?><hello $DEFAULTONLY><capabilities><capability>urn:ietf:params:netconf:base:1.0</capability><capability>urn:ietf:params:netconf:base:1.1</capability></capabilities></hello>]]>]]>"
 
 # XXX cannot get this to work for all combinations of nc/netcat fcgi/native
 # But leave it here for debugging where netcat works properly

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 "<?xml version=\"1.0\" encoding=\"UTF-8\"?><hello $DEFAULTNS><capabilities><capability>urn:ietf:params:netconf:base:1.1?arg=val</capability></capabilities></hello>]]>]]>" '^$'
 
 new "Netconf hello with wrong namespace -> terminate"
-expecteof "$clixon_netconf -qef $cfg" 255 "<?xml version=\"1.0\" encoding=\"UTF-8\"?><hello xmlns='urn:xxx:wrong' message-id=\"42\"><capabilities><capability>urn:ietf:params:netconf:base:1.1</capability></capabilities></hello>]]>]]>" '^$' 2> /dev/null
+expecteof "$clixon_netconf -qef $cfg" 255 "<?xml version=\"1.0\" encoding=\"UTF-8\"?><hello xmlns='urn:xxx:wrong'><capabilities><capability>urn:ietf:params:netconf:base:1.1</capability></capabilities></hello>]]>]]>" '^$' 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 "<?xml version=\"1.0\" encoding=\"UTF-8\"?><nc:hello xmlns:nc=\"urn:ietf:params:xml:ns:netconf:base:1.0\"><nc:capabilities><nc:capability>urn:ietf:params:netconf:base:1.1</nc:capability></nc:capabilities></nc:hello>]]>]]>" '^$'
 
 new "netconf snd + rcv hello"
-expecteof "$clixon_netconf -f $cfg" 0 "<?xml version=\"1.0\" encoding=\"UTF-8\"?><hello $DEFAULTNS><capabilities><capability>urn:ietf:params:netconf:base:1.1</capability></capabilities></hello>]]>]]>" "^<hello $DEFAULTNS><capabilities><capability>urn:ietf:params:netconf:base:1.1</capability><capability>urn:ietf:params:netconf:base:1.0</capability><capability>urn:ietf:params:netconf:capability:yang-library:1.0?revision=2019-01-04&amp;module-set-id=42</capability><capability>urn:ietf:params:netconf:capability:candidate:1.0</capability><capability>urn:ietf:params:netconf:capability:validate:1.1</capability><capability>urn:ietf:params:netconf:capability:startup:1.0</capability><capability>urn:ietf:params:netconf:capability:xpath:1.0</capability><capability>urn:ietf:params:netconf:capability:notification:1.0</capability><capability>urn:ietf:params:netconf:capability:with-defaults:1.0?basic-mode=explicit&amp;also-supported=report-all,trim,report-all-tagged</capability></capabilities><session-id>[0-9]*</session-id></hello>]]>]]>$" '^$'
+expecteof "$clixon_netconf -f $cfg" 0 "<?xml version=\"1.0\" encoding=\"UTF-8\"?><hello $DEFAULTONLY><capabilities><capability>urn:ietf:params:netconf:base:1.1</capability></capabilities></hello>]]>]]>" "^<hello $DEFAULTONLY><capabilities><capability>urn:ietf:params:netconf:base:1.1</capability><capability>urn:ietf:params:netconf:base:1.0</capability><capability>urn:ietf:params:netconf:capability:yang-library:1.0?revision=2019-01-04&amp;module-set-id=42</capability><capability>urn:ietf:params:netconf:capability:candidate:1.0</capability><capability>urn:ietf:params:netconf:capability:validate:1.1</capability><capability>urn:ietf:params:netconf:capability:startup:1.0</capability><capability>urn:ietf:params:netconf:capability:xpath:1.0</capability><capability>urn:ietf:params:netconf:capability:notification:1.0</capability><capability>urn:ietf:params:netconf:capability:with-defaults:1.0?basic-mode=explicit&amp;also-supported=report-all,trim,report-all-tagged</capability></capabilities><session-id>[0-9]*</session-id></hello>]]>]]>$" '^$'
 
+# 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 "<hello $DEFAULTNS><capabilities><extra-element/><capability>urn:ietf:params:netconf:base:1.1</capability></capabilities></hello>]]>]]>" '^<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0" message-id="42"><rpc-error><error-type>protocol</error-type><error-tag>unknown-element</error-tag><error-info><bad-element>extra-element</bad-element></error-info><error-severity>error</error-severity><error-message>Unrecognized hello/capabilities element</error-message></rpc-error></rpc-reply>]]>]]>$' '^$'
+expecteof "$clixon_netconf -qef $cfg" 0 "<hello $DEFAULTONLY><capabilities><extra-element/><capability>urn:ietf:params:netconf:base:1.1</capability></capabilities></hello>]]>]]>" '^<rpc-reply xmlns="urn:ietf:params:xml:ns:netconf:base:1.0"><rpc-error><error-type>protocol</error-type><error-tag>unknown-element</error-tag><error-info><bad-element>extra-element</bad-element></error-info><error-severity>error</error-severity><error-message>Unrecognized hello/capabilities element</error-message></rpc-error></rpc-reply>]]>]]>$' '^$'
 
 new "Netconf send rpc without hello error"
 expecteof "$clixon_netconf -qef $cfg" 255 "<rpc $DEFAULTNS><get-config><source><candidate/></source></get-config></rpc>]]>]]>" "<rpc-reply $DEFAULTNS><rpc-error><error-type>rpc</error-type><error-tag>operation-failed</error-tag><error-severity>error</error-severity><error-message>Client must send an hello element before any RPC</error-message></rpc-error></rpc-reply>]]>]]>" '^$'

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 "<hello $DEFAULTNS><capabilities><capability>urn:ietf:params:netconf:base:1.1</capability><capability>urn:ietf:params:netconf:base:1.0</capability><capability>urn:ietf:params:netconf:capability:yang-library:1.0?revision=2019-01-04&amp;module-set-id=42</capability><capability>urn:ietf:params:netconf:capability:candidate:1.0</capability><capability>urn:ietf:params:netconf:capability:validate:1.1</capability><capability>urn:ietf:params:netconf:capability:startup:1.0</capability><capability>urn:ietf:params:netconf:capability:xpath:1.0</capability><capability>urn:ietf:params:netconf:capability:notification:1.0</capability><capability>urn:ietf:params:netconf:capability:with-defaults:1.0?basic-mode=explicit&amp;also-supported=report-all,trim,report-all-tagged</capability></capabilities><session-id>2</session-id></hello>" "<rpc-reply $DEFAULTNS><data/></rpc-reply>"
+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 "<hello $DEFAULTONLY><capabilities><capability>urn:ietf:params:netconf:base:1.1</capability><capability>urn:ietf:params:netconf:base:1.0</capability><capability>urn:ietf:params:netconf:capability:yang-library:1.0?revision=2019-01-04&amp;module-set-id=42</capability><capability>urn:ietf:params:netconf:capability:candidate:1.0</capability><capability>urn:ietf:params:netconf:capability:validate:1.1</capability><capability>urn:ietf:params:netconf:capability:startup:1.0</capability><capability>urn:ietf:params:netconf:capability:xpath:1.0</capability><capability>urn:ietf:params:netconf:capability:notification:1.0</capability><capability>urn:ietf:params:netconf:capability:with-defaults:1.0?basic-mode=explicit&amp;also-supported=report-all,trim,report-all-tagged</capability></capabilities><session-id>2</session-id></hello>" "<rpc-reply $DEFAULTNS><data/></rpc-reply>"
 
 # Wait 
 wait

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 "<hello $DEFAULTNS/>" "<hello $DEFAULTNS><session-id>3</session-id></hello>"
+    expecteof "$clixon_util_socket -a $family -s $sock -D $DBG" 0 "<hello $DEFAULTONLY/>" "<hello $DEFAULTONLY><session-id>3</session-id></hello>"
 
     new "hello session-id 2"
-    expecteof "$clixon_util_socket -a $family -s $sock -D $DBG" 0 "<hello $DEFAULTNS/>" "<hello $DEFAULTNS><session-id>4</session-id></hello>"
+    expecteof "$clixon_util_socket -a $family -s $sock -D $DBG" 0 "<hello $DEFAULTONLY/>" "<hello $DEFAULTONLY><session-id>4</session-id></hello>"
 
     if [ $BE -ne 0 ]; then
 	new "Kill backend"