From 000cb866c2bab858f945f37d24f3112f0de23f7e Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Sun, 15 Jan 2023 22:29:08 +0100 Subject: [PATCH] RFC 8525: - Change from RFC 7805: Remove revision if empty instead of sending empty revision RFC 6022 - Added cli identity to RFC6022 transport - Added source-host for natove restconf, bit no other sessions --- CHANGELOG.md | 8 ++++-- apps/backend/backend_client.c | 11 +++++--- apps/cli/cli_main.c | 14 +++++----- apps/restconf/restconf_main_fcgi.c | 6 +++++ apps/restconf/restconf_main_native.c | 36 +++++++++++++++++++++++--- apps/restconf/restconf_native.h | 2 ++ apps/snmp/snmp_main.c | 2 +- lib/src/clixon_yang_module.c | 10 ++++--- test/test_augment_state.sh | 1 + test/test_feature.sh | 3 ++- test/test_netconf.sh | 1 + test/test_perf_state.sh | 18 ++++++++----- test/test_perf_state_only.sh | 2 +- test/test_restconf_jukebox.sh | 3 ++- test/test_snmp_ifmib.sh | 6 +++++ test/test_submodule.sh | 5 ++-- yang/clixon/clixon-lib@2022-12-01.yang | 5 ++++ 17 files changed, 100 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bdf4c0df..e04bb037 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,11 +45,13 @@ Expected: beginning of 2023 * Netconf monitoring, part 2 * Datastores and sessions - * Remaining: statistics state + * Added clixon-specific transport identities: cli, snmp, netconf, restconf + * Added source-host fro native restonf, but no other transports * Standards * RFC 6022 "YANG Module for NETCONF Monitoring" * See [Feature Request: Support RFC 6022 (NETCONF Monitoring)](https://github.com/clicon/clixon/issues/370) - + * Remaining: statistics state + ### API changes on existing protocol/config features Users may have to change how they access the system @@ -87,6 +89,8 @@ Developers may need to change their code ### Corrected Bugs +* Fixed: yang-library: Remove revision if empty instead of sending empty revision + * This was a change from RFC 7895 to RFC 8525 * Fixed: [locally scoped YANG typedef in grouping does not work #394](https://github.com/clicon/clixon/issues/394) * Fixed: [leafref in new type no work in union type](https://github.com/clicon/clixon/issues/388) * Fixed: [must statement check int value failed](https://github.com/clicon/clixon/issues/386) diff --git a/apps/backend/backend_client.c b/apps/backend/backend_client.c index fb50c364..33b0830c 100644 --- a/apps/backend/backend_client.c +++ b/apps/backend/backend_client.c @@ -192,10 +192,13 @@ backend_monitoring_state_get(clicon_handle h, for (ce = backend_client_list(h); ce; ce = ce->ce_next){ cprintf(cb, ""); cprintf(cb, "%u", ce->ce_id); - if (ce->ce_transport) - cprintf(cb, "%s", - CLIXON_LIB_PREFIX, CLIXON_LIB_NS, - ce->ce_transport); + if (ce->ce_transport == NULL){ + clicon_err(OE_XML, 0, "Mandatory element transport missing"); + goto done; + } + cprintf(cb, "%s", + CLIXON_LIB_PREFIX, CLIXON_LIB_NS, + ce->ce_transport); cprintf(cb, "%s", ce->ce_username); if (ce->ce_source_host) cprintf(cb, "%s", ce->ce_source_host); diff --git a/apps/cli/cli_main.c b/apps/cli/cli_main.c index a7289197..0ecdb3fa 100644 --- a/apps/cli/cli_main.c +++ b/apps/cli/cli_main.c @@ -167,7 +167,8 @@ cli_terminate(clicon_handle h) cvec *nsctx; cxobj *x; - clicon_rpc_close_session(h); + if (clicon_data_get(h, "session-transport", NULL) == 0) + clicon_rpc_close_session(h); if ((yspec = clicon_dbspec_yang(h)) != NULL) ys_free(yspec); if ((yspec = clicon_config_yang(h)) != NULL) @@ -805,6 +806,12 @@ main(int argc, goto done; /* Experimental utf8 mode */ cligen_utf8_set(cli_cligen(h), clicon_option_int(h,"CLICON_CLI_UTF8")); + + /* Set RFC6022 session parameters that will be sent in first hello, + * @see clicon_hello_req + */ + clicon_data_set(h, "session-transport", "cl:cli"); + /* Launch interfactive event loop, unless -1 */ if (restarg != NULL && strlen(restarg)){ char *mode = cli_syntax_mode(h); @@ -818,11 +825,6 @@ main(int argc, if (evalresult < 0) goto done; } - /* Set RFC6022 session parameters that will be sent in first hello, - * @see clicon_hello_req - */ - clicon_data_set(h, "session-transport", "cl:cli"); - clicon_data_set(h, "session-source-host", "localhost"); /* Go into event-loop unless -1 command-line */ if (!once){ diff --git a/apps/restconf/restconf_main_fcgi.c b/apps/restconf/restconf_main_fcgi.c index 48251057..692cf2df 100644 --- a/apps/restconf/restconf_main_fcgi.c +++ b/apps/restconf/restconf_main_fcgi.c @@ -572,11 +572,17 @@ main(int argc, clicon_err(OE_UNIX, errno, "chmod"); goto done; } + /* Drop privileges if started as root to CLICON_RESTCONF_USER * and use drop mode: CLICON_RESTCONF_PRIVILEGES */ if (restconf_drop_privileges(h) < 0) goto done; + /* Set RFC6022 session parameters that will be sent in first hello, + * @see clicon_hello_req + */ + clicon_data_set(h, "session-transport", "cl:restconf"); + if (FCGX_InitRequest(req, sock, 0) != 0){ clicon_err(OE_CFG, errno, "FCGX_InitRequest"); goto done; diff --git a/apps/restconf/restconf_main_native.c b/apps/restconf/restconf_main_native.c index f6bd11b0..b3f0ca31 100644 --- a/apps/restconf/restconf_main_native.c +++ b/apps/restconf/restconf_main_native.c @@ -540,22 +540,43 @@ restconf_accept_client(int fd, struct sockaddr from = {0,}; socklen_t len; char *name = NULL; + void *addr; clicon_debug(1, "%s %d", __FUNCTION__, fd); if ((rsock = (restconf_socket *)arg) == NULL){ clicon_err(OE_YANG, EINVAL, "rsock is NULL"); goto done; } - clicon_debug(1, "%s type:%s addr:%s port:%hu", __FUNCTION__, - rsock->rs_addrtype, - rsock->rs_addrstr, - rsock->rs_port); h = rsock->rs_h; len = sizeof(from); if ((s = accept(rsock->rs_ss, &from, &len)) < 0){ clicon_err(OE_UNIX, errno, "accept"); goto done; } + switch (from.sa_family){ + case AF_INET:{ + struct sockaddr_in *in = (struct sockaddr_in *)&from; + addr = &(in->sin_addr); + break; + } + case AF_INET6:{ + struct sockaddr_in6 *in6 = (struct sockaddr_in6 *)&from; + addr = &(in6->sin6_addr); + break; + } + } + if ((rsock->rs_from_addr = calloc(INET6_ADDRSTRLEN, 1)) == NULL){ + clicon_err(OE_UNIX, errno, "calloc"); + goto done; + } + if (inet_ntop(from.sa_family, addr, rsock->rs_from_addr, INET6_ADDRSTRLEN) < 0) + goto done; + clicon_debug(1, "%s type:%s from:%s, dest:%s port:%hu", __FUNCTION__, + rsock->rs_addrtype, + rsock->rs_from_addr, + rsock->rs_addrstr, + rsock->rs_port); + clicon_data_set(h, "session-source-host", rsock->rs_from_addr); /* Accept SSL */ if (restconf_ssl_accept_client(h, s, rsock, NULL) < 0) goto done; @@ -601,6 +622,8 @@ restconf_native_terminate(clicon_handle h) free(rsock->rs_addrstr); if (rsock->rs_addrtype) free(rsock->rs_addrtype); + if (rsock->rs_from_addr) + free(rsock->rs_from_addr); free(rsock); } if (rn->rn_ctx) @@ -1285,6 +1308,11 @@ main(int argc, if (restconf_drop_privileges(h) < 0) goto done; + /* Set RFC6022 session parameters that will be sent in first hello, + * @see clicon_hello_req + */ + clicon_data_set(h, "session-transport", "cl:restconf"); + /* Main event loop */ if (clixon_event_loop(h) < 0) goto done; diff --git a/apps/restconf/restconf_native.h b/apps/restconf/restconf_native.h index 38e1a149..3b270fc0 100644 --- a/apps/restconf/restconf_native.h +++ b/apps/restconf/restconf_native.h @@ -155,6 +155,8 @@ typedef struct restconf_socket{ * Set in restconf_callhome_cb */ restconf_conn *rs_conns; /* List of transient connect sockets */ + char *rs_from_addr; /* From IP address as seen by accept */ + } restconf_socket; /* Restconf handle diff --git a/apps/snmp/snmp_main.c b/apps/snmp/snmp_main.c index ac3bc04a..0d3a0e99 100644 --- a/apps/snmp/snmp_main.c +++ b/apps/snmp/snmp_main.c @@ -525,7 +525,7 @@ main(int argc, * used by the client, even though new TCP sessions are created for * each message sent to the backend. */ - if (clicon_hello_req(h, "cl:snmp", "localhost", &id) < 0) + if (clicon_hello_req(h, "cl:snmp", NULL, &id) < 0) goto done; clicon_session_id_set(h, id); diff --git a/lib/src/clixon_yang_module.c b/lib/src/clixon_yang_module.c index be0bfbd5..db2d0981 100644 --- a/lib/src/clixon_yang_module.c +++ b/lib/src/clixon_yang_module.c @@ -239,8 +239,12 @@ yms_build(clicon_handle h, cprintf(cb,"%s", yang_argument_get(ys)); else{ /* RFC7895 1 If no (such) revision statement exists, the module's or - submodule's revision is the zero-length string. */ - cprintf(cb,""); + submodule's revision is the zero-length string. + But in RFC8525 this has changed to: + If no revision statement is present in the YANG module or submodule, this + leaf is not instantiated. + cprintf(cb,""); + */ } if ((ys = yang_find(ymod, Y_NAMESPACE, NULL)) != NULL) cprintf(cb,"%s", yang_argument_get(ys)); @@ -273,8 +277,6 @@ yms_build(clicon_handle h, if ((ysub = yang_find(yspec, Y_SUBMODULE, name)) != NULL){ if ((ys = yang_find(ysub, Y_REVISION, NULL)) != NULL) cprintf(cb,"%s", yang_argument_get(ys)); - else - cprintf(cb,""); } cprintf(cb,""); } diff --git a/test/test_augment_state.sh b/test/test_augment_state.sh index 68de7081..142ca9f3 100755 --- a/test/test_augment_state.sh +++ b/test/test_augment_state.sh @@ -29,6 +29,7 @@ cat < $cfg false false false + true EOF diff --git a/test/test_feature.sh b/test/test_feature.sh index 92221390..a4f6b94a 100755 --- a/test/test_feature.sh +++ b/test/test_feature.sh @@ -41,6 +41,7 @@ cat < $cfg /usr/local/var/$APPNAME/$APPNAME.sock /usr/local/var/$APPNAME/$APPNAME.pidfile /usr/local/var/$APPNAME + true EOF @@ -235,7 +236,7 @@ if [ -z "$match" ]; then fi new "netconf module A" -expect="exampleurn:example:clixonAA1" +expect="exampleurn:example:clixonAA1" match=`echo "$ret" | grep --null -Go "$expect"` if [ -z "$match" ]; then err "$expect" "$ret" diff --git a/test/test_netconf.sh b/test/test_netconf.sh index 758c4baa..c50416c1 100755 --- a/test/test_netconf.sh +++ b/test/test_netconf.sh @@ -36,6 +36,7 @@ cat < $cfg $dir/$APPNAME.sock /usr/local/var/$APPNAME/$APPNAME.pidfile /usr/local/var/$APPNAME + true EOF diff --git a/test/test_perf_state.sh b/test/test_perf_state.sh index 970fd60a..97b391b3 100755 --- a/test/test_perf_state.sh +++ b/test/test_perf_state.sh @@ -50,6 +50,7 @@ cat < $cfg /usr/local/lib/example/clispec 0 ietf-netconf:startup + true $RESTCONFIG EOF @@ -121,20 +122,23 @@ if [ $RC -ne 0 ]; then fi new "wait restconf" -wait_restconf +swait_restconf rpc="" rpc+="foo" for (( i=0; i<$perfnr; i++ )); do - rpc+="e$iex:eth" + rpc+="e$ieth" done rpc+="" -rpc+="$(cat $fconfig)" +#rpc+="$(cat $fstate)" rpc+="" -echo foo + echo -n "$DEFAULTHELLO" > $fconfig echo "$(chunked_framing "$rpc")" >> $fconfig +echo "fstate:$fstate" +echo "fconfig:$fconfig" + # Now take large config file and write it via netconf to candidate new "netconf write large config" expecteof_file "time -p $clixon_netconf -qef $cfg" 0 "$fconfig" "^$" 2>&1 | awk '/real/ {print $2}' @@ -150,7 +154,7 @@ new "netconf get test single req" sel="/ex:interfaces/ex:a[ex:name='foo']/ex:b/ex:interface[ex:name='e1']" rpc=$(chunked_framing "") -expecteof_netconf "$clixon_netconf -qef $cfg" 0 "$DEFAULTHELLO" "$rpc" "" "fooe1ex:ethup" +expecteof_netconf "$clixon_netconf -qef $cfg" 0 "$DEFAULTHELLO" "$rpc" "" "fooe1ethup" new "netconf get $perfreq single reqs" { time -p for (( i=0; i<$perfreq; i++ )); do @@ -162,7 +166,7 @@ done | $clixon_netconf -qe1f $cfg > /dev/null; } 2>&1 | awk '/real/ {print $2}' # RESTCONF get new "restconf get test single req" -expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/example:interfaces/a=foo/b/interface=e1)" 0 "HTTP/$HVER 200" '{"example:interface":\[{"name":"e1","type":"ex:eth","status":"up"}\]}' +expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/example:interfaces/a=foo/b/interface=e1)" 0 "HTTP/$HVER 200" '{"example:interface":\[{"name":"e1","type":"eth","status":"up"}\]}' new "restconf get $perfreq single reqs" #curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/ietf-interfaces:interfaces/interface=e67 @@ -178,7 +182,7 @@ edit interfaces a foo b interface e1 show state xml EOF new "cli get test single req" -expectpart "$($clixon_cli -F $fin -f $cfg)" 0 "e1" "ex:eth" "up$" +expectpart "$($clixon_cli -F $fin -f $cfg)" 0 "e1" "eth" "up$" new "cli get $perfreq single reqs" { time -p for (( i=0; i<$perfreq; i++ )); do diff --git a/test/test_perf_state_only.sh b/test/test_perf_state_only.sh index 98b76258..3c4b2670 100755 --- a/test/test_perf_state_only.sh +++ b/test/test_perf_state_only.sh @@ -26,7 +26,6 @@ APPNAME=example cfg=$dir/config.xml fyang=$dir/$APPNAME.yang -fconfig=$dir/large.xml fstate=$dir/state.xml # Define default restconfig config: RESTCONFIG @@ -49,6 +48,7 @@ cat < $cfg /usr/local/lib/example/cli /usr/local/lib/example/clispec ietf-netconf:startup + true $RESTCONFIG EOF diff --git a/test/test_restconf_jukebox.sh b/test/test_restconf_jukebox.sh index d596002b..b362f904 100755 --- a/test/test_restconf_jukebox.sh +++ b/test/test_restconf_jukebox.sh @@ -40,6 +40,7 @@ cat < $cfg true clixon-restconf:fcgi true + true $RESTCONFIG EOF @@ -106,7 +107,7 @@ expectpart "$(curl $CURLOPTS -X GET -H 'Accept: application/yang-data+xml' $RCPR # This just catches the header and the jukebox module, the RFC has foo and bar which # seems wrong to recreate new "B.1.2. Retrieve the Server Module Information" -expectpart "$(curl $CURLOPTS -X GET -H 'Accept: application/yang-data+json' $RCPROTO://localhost/restconf/data/ietf-yang-library:yang-library)" 0 "HTTP/$HVER 200" 'Cache-Control: no-cache' "Content-Type: application/yang-data+json" "{\"ietf-yang-library:yang-library\":{\"module-set\":\[{\"name\":\"default\",\"module\":\[{\"name\":\"clixon-lib\",\"revision\":\"${CLIXON_LIB_REV}\",\"namespace\":\"http://clicon.org/lib\"}" '{"name":"example-events","revision":"","namespace":"urn:example:events"' '{"name":"example-jukebox","revision":"2016-08-15","namespace":"http://example.com/ns/example-jukebox"}' '{"name":"example-system","revision":"","namespace":"http://example.com/ns/example-system"}' +expectpart "$(curl $CURLOPTS -X GET -H 'Accept: application/yang-data+json' $RCPROTO://localhost/restconf/data/ietf-yang-library:yang-library)" 0 "HTTP/$HVER 200" 'Cache-Control: no-cache' "Content-Type: application/yang-data+json" "{\"ietf-yang-library:yang-library\":{\"module-set\":\[{\"name\":\"default\",\"module\":\[{\"name\":\"clixon-lib\",\"revision\":\"${CLIXON_LIB_REV}\",\"namespace\":\"http://clicon.org/lib\"}" '{"name":"example-events","namespace":"urn:example:events"' '{"name":"example-jukebox","revision":"2016-08-15","namespace":"http://example.com/ns/example-jukebox"}' '{"name":"example-system","namespace":"http://example.com/ns/example-system"}' new "B.1.3. Retrieve the Server Capability Information" expectpart "$(curl $CURLOPTS -X GET -H 'Accept: application/yang-data+xml' $RCPROTO://localhost/restconf/data/ietf-restconf-monitoring:restconf-state/capabilities)" 0 "HTTP/$HVER 200" "Content-Type: application/yang-data+xml" 'Cache-Control: no-cache' 'urn:ietf:params:restconf:capability:defaults:1.0?basic-mode=expliciturn:ietf:params:restconf:capability:depth diff --git a/test/test_snmp_ifmib.sh b/test/test_snmp_ifmib.sh index 6ab5aa83..5dc419b6 100755 --- a/test/test_snmp_ifmib.sh +++ b/test/test_snmp_ifmib.sh @@ -422,6 +422,12 @@ expectpart "$($snmpwalk IF-MIB::ifTable)" 0 "IF-MIB::ifIndex.1 = INTEGER: 1" \ "IF-MIB::ifSpecific.1 = OID: SNMPv2-SMI::zeroDotZero" \ "IF-MIB::ifSpecific.2 = OID: iso.2.3" + +# There is an intricate error in the return of this test that has to with validation of state data +# clixon_snmp queries using xpath: +# if-mib:IF-MIB/if-mib:ifRcvAddressTable/if-mib:ifRcvAddressEntry[if-mib:ifIndex='1'] +# But if-mib:ifIndex is a leafref that point to a value outside the XPath, which make the validation fail +# But if you set CLICON_VALIDATE_STATE_XML:true its ok new "Test $OID24" validate_oid $OID24 $OID24 "STRING" "11:bb:cc:dd:ee:ff" validate_oid $NAME24 $NAME24 "STRING" "11:bb:cc:dd:ee:ff" "IF-MIB::ifRcvAddressAddress.1.\"11:bb:cc:dd:ee:ff\" = STRING: 11:bb:cc:dd:ee:ff" diff --git a/test/test_submodule.sh b/test/test_submodule.sh index e6328b39..d2a08c71 100755 --- a/test/test_submodule.sh +++ b/test/test_submodule.sh @@ -47,6 +47,7 @@ cat < $cfg /usr/local/lib/$APPNAME/restconf /usr/local/var/$APPNAME true + true $RESTCONFIG EOF @@ -263,10 +264,10 @@ new "restconf edit augment 2" expectpart "$(curl $CURLOPTS -X POST -H "Content-Type: application/yang-data+json" $RCPROTO://localhost/restconf/data/main:sub2 -d '{"main:aug2":"foo"}')" 0 "HTTP/$HVER 201" new "NETCONF get module state" -expecteof_netconf "$clixon_netconf -qf $cfg -D $DBG" 0 "$DEFAULTHELLO" "" "defaultmain2021-03-08urn:example:clixonsub1A" "" +expecteof_netconf "$clixon_netconf -qf $cfg -D $DBG" 0 "$DEFAULTHELLO" "" "defaultmain2021-03-08urn:example:clixonsub1A" "" new "RESTCONF get module state" -expectpart "$(curl $CURLOPTS -X GET -H "Accept: application/yang-data+xml" $RCPROTO://localhost/restconf/data/ietf-yang-library:yang-library/module-set=default/module=main?config=nonconfig)" 0 "HTTP/$HVER 200" "main2021-03-08urn:example:clixonsub1A" +expectpart "$(curl $CURLOPTS -X GET -H "Accept: application/yang-data+xml" $RCPROTO://localhost/restconf/data/ietf-yang-library:yang-library/module-set=default/module=main?config=nonconfig)" 0 "HTTP/$HVER 200" "main2021-03-08urn:example:clixonsub1A" if [ $RC -ne 0 ]; then new "Kill restconf daemon" diff --git a/yang/clixon/clixon-lib@2022-12-01.yang b/yang/clixon/clixon-lib@2022-12-01.yang index d17368b1..05ceb9b6 100644 --- a/yang/clixon/clixon-lib@2022-12-01.yang +++ b/yang/clixon/clixon-lib@2022-12-01.yang @@ -148,6 +148,11 @@ module clixon-lib { "RESTCONF either as HTTP/1 or /2, TLS or not, reverese proxy (eg fcgi/nginx) or native"; base ncm:transport; } + identity cli { + description + "A CLI session"; + base ncm:transport; + } extension autocli-op { description "Takes an argument an operation defing how to modify the clispec at