Added validation of clixon-restconf.yang: server-key-path and server-cert-path must be present if ssl enabled.

CHANGELOG.md

@@ -53,6 +53,8 @@ Users may have to change how they access the system
 
 ### Minor changes
 
+* Added validation of clixon-restconf.yang: server-key-path and server-cert-path must be present if ssl enabled.
+  * Only if `CLICON_BACKEND_RESTCONF_PROCESS` is true
 * Experimental IPC API, `clixon_client`, to support a loose integration model
   * Many systems using other tools employ such a model, and this API is an effort to make a usage of clixon easier
   * see https://clixon-docs.readthedocs.io/en/latest/overview.html#loose-integration
@@ -338,7 +340,7 @@ Users may have to change how they access the system
 Developers may need to change their code
 
 * Added yang-binding `yb` parameter to `xmldb_get0()` and all xmldb get functions.
-* Simplified the _module-specific upgrade API.
+* Simplified the _module-specific_ upgrade API.
   * The new API is documented here: [Module-specific upgrade](https://clixon-docs.readthedocs.io/en/latest/upgrade.html#module-specific-upgrade)
   * The change is not backward compatible. The API has been simplified which means more has to be done by the programmer.
   * In summary, a user registers an upgrade callback per module. The callback is called at startup if the module is added, has been removed or if the revision on file is different from the one in the system. 

apps/backend/backend_main.c

@@ -474,6 +474,35 @@ restconf_pseudo_process_control(clicon_handle h)
     return retval;
 }
 
+/*! Restconf pseduo-plugin process validate
+ */
+static int
+restconf_pseudo_process_validate(clicon_handle    h,
+				 transaction_data td)
+{
+    int    retval = -1;
+    cxobj *xtarget;
+
+    clicon_debug(1, "%s", __FUNCTION__);
+    xtarget = transaction_target(td);
+    /* If ssl-enable is true and (at least a) socket has ssl,
+     * then server-cert-path and server-key-path must exist */
+    if (xpath_first(xtarget, NULL, "restconf/enable[.='true']") &&
+	xpath_first(xtarget, NULL, "restconf/socket[ssl='true']")){
+	/* Should filepath be checked? One could claim this is a runtime system,... */
+	if (xpath_first(xtarget, 0, "restconf/server-cert-path") == NULL){
+	    clicon_err(OE_CFG, 0, "SSL enabled but server-cert-path not set");
+	    return -1; /* induce fail */
+	}
+	if (xpath_first(xtarget, 0, "restconf/server-key-path") == NULL){
+	    clicon_err(OE_CFG, 0, "SSL enabled but server-key-path not set");
+	    return -1; /* induce fail */
+	}
+    }
+    retval = 0;
+    return retval;
+}
+
 /*! Restconf pseduo-plugin process commit
  */
 static int
@@ -513,6 +542,7 @@ restconf_pseudo_process_reg(clicon_handle h,
     if (clixon_pseudo_plugin(h, "restconf pseudo plugin", &cp) < 0)
 	goto done;
     cp->cp_api.ca_trans_commit = restconf_pseudo_process_commit;
+    cp->cp_api.ca_trans_validate = restconf_pseudo_process_validate;
 
     /* Register generic process-control of restconf daemon, ie start/stop restconf */
     if (restconf_pseudo_process_control(h) < 0)

apps/restconf/restconf_main_evhtp.c

@@ -530,8 +530,16 @@ cx_get_ssl_server_certs(clicon_handle    h,
     int         retval = -1;
     struct stat f_stat;
 
-    if (ssl_config == NULL || server_cert_path == NULL){
+    if (ssl_config == NULL){
-	clicon_err(OE_CFG, EINVAL, "Input parameter is NULL");
+	clicon_err(OE_CFG, EINVAL, "ssl_config is NULL");
+	goto done;
+    }
+    if (server_cert_path == NULL){
+	clicon_err(OE_CFG, EINVAL, "server_cert_path is not set but is required when ssl is enabled");
+	goto done;
+    }
+    if (server_key_path == NULL){
+	clicon_err(OE_CFG, EINVAL, "server_key_path is not set but is required when ssl is enabled");
 	goto done;
     }
     if ((ssl_config->pemfile = strdup(server_cert_path)) == NULL){
@@ -695,7 +703,6 @@ restconf_socket_init(clicon_handle h,
     //    return evhtp_bind_sockaddr(htp, sa, sin_len, SOCKET_LISTEN_BACKLOG);
 }
 
-
 /*! Usage help routine
  * @param[in]  argv0  command line
  * @param[in]  h      Clicon handle

test/test_client.sh

@@ -0,0 +1,166 @@
+#!/usr/bin/env bash
+# Advanced Client api test
+# Compile and run a client
+
+# Magic line must be first in script (see README.md)
+s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
+
+# Which format to use as datastore format internally
+: ${format:=xml}
+
+APPNAME=example
+
+cfg=$dir/conf_yang.xml
+fyang=$dir/example-client.yang
+cfile=$dir/example-client.c
+pdir=$dir/plugin
+app=$pdir/example-api
+
+if [ ! -d $pdir ]; then
+    mkdir $pdir
+fi
+
+cat <<EOF > $cfg
+<clixon-config xmlns="http://clicon.org/config">
+  <CLICON_CONFIGFILE>$cfg</CLICON_CONFIGFILE>
+  <CLICON_YANG_DIR>/usr/local/share/clixon</CLICON_YANG_DIR>
+  <CLICON_YANG_DIR>$IETFRFC</CLICON_YANG_DIR>
+  <CLICON_YANG_MAIN_FILE>$fyang</CLICON_YANG_MAIN_FILE>
+  <CLICON_CLISPEC_DIR>/usr/local/lib/$APPNAME/clispec</CLICON_CLISPEC_DIR>
+  <CLICON_CLI_DIR>/usr/local/lib/$APPNAME/cli</CLICON_CLI_DIR>
+  <CLICON_CLI_MODE>$APPNAME</CLICON_CLI_MODE>
+  <CLICON_SOCK>/usr/local/var/$APPNAME/$APPNAME.sock</CLICON_SOCK>
+  <CLICON_RESTCONF_PRETTY>false</CLICON_RESTCONF_PRETTY>
+  <CLICON_BACKEND_DIR>$pdir</CLICON_BACKEND_DIR>
+  <CLICON_BACKEND_PIDFILE>/usr/local/var/$APPNAME/$APPNAME.pidfile</CLICON_BACKEND_PIDFILE>
+  <CLICON_XMLDB_DIR>$dir</CLICON_XMLDB_DIR>
+  <CLICON_XMLDB_FORMAT>$format</CLICON_XMLDB_FORMAT>
+  $RESTCONFIG
+</clixon-config>
+EOF
+
+cat <<EOF > $fyang
+module clixon-client {
+    yang-version 1.1;
+    namespace "urn:example:clixon-client";
+    prefix exc;
+    description
+	"Clixon client example yang";
+    revision 2021-01-14 {
+        description "Added table/paramater/value as the primary data example";
+    }
+    /* Generic config data */
+    container table{
+      list parameter{
+	    key name;
+	    leaf name{
+		type string;
+	    }
+	    leaf value{
+		type uint32;
+	    }
+	}
+    }
+}
+EOF
+
+cat<<EOF > $cfile
+#include <unistd.h>
+#include <stdio.h>
+#include <sys/socket.h>
+#include <netinet/in.h> /* sockaddr_in */
+#include <arpa/inet.h>  /* inet_addr */
+#include <clixon/clixon_client.h>
+
+#define CLIXONCONF "$cfg"
+
+int
+main(int    argc,
+     char **argv)
+{
+    int   s;
+    void *h = NULL; /* clixon handle */
+
+    if ((h = clixon_client_init("server", stderr, 0, CLIXONCONF)) == NULL)
+       return -1;
+    if ((s = clixon_client_connect(h)) < 0){
+       return -1;
+    }
+    /* Here are read functions depending on an example YANG 
+     * (Need an example YANG and XML input to confd)
+     */
+    {
+       uint32_t u = 0;
+       if (clixon_client_get_uint32(s, &u, "urn:example:clixon-client", "/table/parameter[name='a']/value") < 0)
+          return -1;
+    }
+    clixon_client_close(s);
+    clixon_client_terminate(h);
+    return 0;
+}
+EOF
+
+new "compile $cfile -> $app"
+echo "$CC -g -Wall -I/usr/local/include $cfile -o $app -lclixon"
+expectpart "$($CC -g -Wall -I/usr/local/include $cfile -o $app -lclixon)" 0 ""
+exit
+new "test params: -s init -f $cfg"
+
+if [ $BE -ne 0 ]; then
+    new "kill old backend"
+    sudo clixon_backend -zf $cfg
+    if [ $? -ne 0 ]; then
+	err
+    fi
+    new "start backend"
+    start_backend -s init -f $cfg
+ fi
+
+new "waiting"
+wait_backend
+
+if [ $RC -ne 0 ]; then
+    new "kill old restconf daemon"
+    stop_restconf_pre
+
+    new "start restconf daemon"
+    start_restconf -f $cfg
+
+    new "waiting"
+    wait_restconf
+fi
+
+XML='<c xmlns="urn:example:api"><y3><k>2</k></y3><y3><k>3</k></y3><y3><k>5</k><val>zorro</val></y3><y3><k>7</k></y3></c>'
+
+# Add a set of entries using restconf
+new "PUT a set of entries"
+expectpart "$(curl $CURLOPTS -X PUT -H 'Content-Type: application/yang-data+xml' $RCPROTO://localhost/restconf/data/example-api:c -d "$XML")" 0 "HTTP/1.1 201 Created"
+
+new "Check entries"
+expectpart "$(curl $CURLOPTS -X GET $RCPROTO://localhost/restconf/data/example-api:c -H 'Accept: application/yang-data+xml')" 0 'HTTP/1.1 200 OK' "$XML"
+
+new "Send a trigger"
+expectpart "$(curl $CURLOPTS -X POST $RCPROTO://localhost/restconf/operations/example-api:trigger -H 'Accept: application/yang-data+json')" 0 'HTTP/1.1 204 No Content'
+
+if [ $RC -ne 0 ]; then
+    new "Kill restconf daemon"
+    stop_restconf
+fi
+
+if [ $BE -eq 0 ]; then
+    exit # BE
+fi
+
+new "Kill backend"
+# Check if premature kill
+pid=`pgrep -u root -f clixon_backend`
+if [ -z "$pid" ]; then
+    err "backend already dead"
+fi
+# kill backend
+stop_backend -f $cfg
+
+# unset conditional parameters 
+unset format
+
+rm -rf $dir

test/test_restconf.sh

@@ -106,7 +106,6 @@ fi
 # Restconf test routine with arguments:
 # 1. proto:http/https
 # 2: addr: 127.0.0.1/::1 # IPv4 or IPv6
-# 3: config: local / backend config (evhtp only)
 testrun()
 {
     proto=$1  # http/https

test/test_restconf_rpc.sh

@@ -262,6 +262,14 @@ new "check status RPC off"
 pid=$(testrpc status 0)
 if [ $? -ne 0 ]; then exit -1; fi
 
+# Negative validation checks of clixon-restconf / socket
+
+new "netconf edit config invalid ssl"
+expecteof "$clixon_netconf -qf $cfg" 0 "<rpc $DEFAULTNS><edit-config><target><candidate/></target><config><restconf xmlns=\"http://clicon.org/restconf\" nc:operation=\"replace\" xmlns:nc=\"urn:ietf:params:xml:ns:netconf:base:1.0\"><enable>true</enable><socket><namespace>default</namespace><address>0.0.0.0</address><port>80</port><ssl>true</ssl></socket></restconf></config></edit-config></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><ok/></rpc-reply>]]>]]>$"
+
+new "netconf validate should fail"
+expecteof "$clixon_netconf -qf $cfg" 0 "<rpc $DEFAULTNS><validate><source><candidate/></source></validate></rpc>]]>]]>" "^<rpc-reply $DEFAULTNS><rpc-error><error-type>application</error-type><error-tag>operation-failed</error-tag><error-severity>error</error-severity><error-message>SSL enabled but server-cert-path not set</error-message></rpc-error></rpc-reply>]]>]]>$"
+
 if false; then # Work in progress - namespace
 #-------------------------------
 # Now in a separate network namespace
@@ -314,5 +322,10 @@ sudo ip netns delete $netns
 
 fi # namespaces
 
+unset pid
+sleep $DEMWAIT # Lots of processes need to die before next test
+
+endtest
+
 rm -rf $dir