Restconf: Added fallback mechanism for non-ALPN HTTPS

* Set `CLICON_RESTCONF_NOALPN_DEFAULT` to `http/2` or `http/1.1`
  * For http/1 or http/2 only, that will be the default if no ALPN is set.

CHANGELOG.md

@@ -42,6 +42,15 @@
 ## 6.2.0
 Expected: April 2023
 
+### New features
+
+### API changes on existing protocol/config features
+
+Users may have to change how they access the system
+
+* New `clixon-config@2022-12-01.yang` revision
+  * Added options: `CLICON_RESTCONF_NOALPN_DEFAULT`
+
 ### C/CLI-API changes on existing features
 Developers may need to change their code
 
@@ -51,8 +60,13 @@ Developers may need to change their code
 	
 ### Minor features
 
+* Restconf: Added fallback mechanism for non-ALPN HTTPS
+  * Set `CLICON_RESTCONF_NOALPN_DEFAULT` to `http/2` or `http/1.1`
+  * For http/1 or http/2 only, that will be the default if no ALPN is set.
 * Fixed: [Add support decimal64 for SNMP](https://github.com/clicon/clixon/pull/422)
 
+### Corrected Bugs
+
 ## 6.1.0
 19 Feb 2023
 

apps/restconf/restconf_native.c

@@ -210,7 +210,6 @@ restconf_conn_free(restconf_conn *rc)
         clicon_err(OE_RESTCONF, EINVAL, "rc is NULL");
         goto done;
     }
-    clicon_debug(1, "%s %p", __FUNCTION__, rc);
 #ifdef HAVE_LIBNGHTTP2
     if (rc->rc_ngsession)
         nghttp2_session_del(rc->rc_ngsession);
@@ -715,7 +714,12 @@ restconf_http1_process(restconf_conn *rc,
             cprintf(cberr, "<errors xmlns=\"urn:ietf:params:xml:ns:yang:ietf-restconf\"><error><error-type>protocol</error-type><error-tag>malformed-message</error-tag><error-message>%s</error-message></error></errors>", clicon_err_reason);
             if ((ret = native_send_badrequest(h, "application/yang-data+xml", cbuf_get(cberr), rc)) < 0)
                 goto done;
-            goto ok;
+            if (http1_native_clear_input(h, sd) < 0)
+                goto done;
+            if (restconf_close_ssl_socket(rc, __FUNCTION__, 0) < 0)
+                goto done;
+            rc = NULL;
+            goto closed;
         }
         /* Check for Continue and if so reply with 100 Continue 
          * ret == 1: send reply
@@ -732,8 +736,7 @@ restconf_http1_process(restconf_conn *rc,
                 if (restconf_close_ssl_socket(rc, __FUNCTION__, 0) < 0)
                     goto done;
                 rc = NULL;
-                retval = 0;
+                goto closed;
-                goto done;
             }
         }
     }
@@ -774,8 +777,7 @@ restconf_http1_process(restconf_conn *rc,
     if (ret == 0 || rc->rc_exit){  /* Server-initiated exit */
         if (restconf_close_ssl_socket(rc, __FUNCTION__, 0) < 0)
             goto done;
-        retval = 0;
+        goto closed;
-        goto done;
     }
  ok:
     retval = 1;
@@ -783,6 +785,9 @@ restconf_http1_process(restconf_conn *rc,
     if (cberr)
         cbuf_free(cberr);
     return retval;
+ closed:
+    retval = 0;
+    goto done;
 }
 #endif
 
@@ -982,9 +987,10 @@ restconf_connection(int   s,
         case HTTP_11:
             if ((ret = restconf_http1_process(rc, buf, n, &readmore)) < 0)
                 goto done;
-            gettimeofday(&rc->rc_t, NULL); /* activity timer */
+            if (ret == 0){
-            if (ret == 0)
                 goto ok;
+            }
+            gettimeofday(&rc->rc_t, NULL); /* activity timer */
             if (readmore)
                 continue;
 #ifdef HAVE_LIBNGHTTP2
@@ -1126,14 +1132,10 @@ ssl_alpn_check(clicon_handle        h,
     /* Alternatively, call  restconf_str2proto but alpn is not a proper string */
     if (alpn && alpnlen == 8 && memcmp("http/1.1", alpn, 8) == 0){
         *proto = HTTP_11;
-        retval = 1; /* http/1.1 */
-        goto done; 
     }
 #ifdef HAVE_LIBNGHTTP2
     else if (alpn && alpnlen == 2 && memcmp("h2", alpn, 2) == 0){
         *proto = HTTP_2;
-        retval = 1; /* http/2 */
-        goto done;
     }
 #endif
     else {
@@ -1148,20 +1150,43 @@ ssl_alpn_check(clicon_handle        h,
                                        "application/yang-data+xml",
                                        cbuf_get(cberr), rc) < 0)
                 goto done;
-        }
-        else{
-            /* XXX Sending badrequest here gives a segv in SSL_shutdown() later or a SIGPIPE here */
-            clicon_log(LOG_INFO, "%s Warning: ALPN: No protocol selected, or no ALPN?", __FUNCTION__);
-        }
             if (restconf_close_ssl_socket(rc, __FUNCTION__, 0) < 0)
                 goto done;
+            goto fail;
         }
-    retval = 0; /* ALPN not OK */
+        else{
+#if defined(HAVE_HTTP1)
+#if defined(HAVE_LIBNGHTTP2)
+            char *pstr; /* Both http/1 and http/2 */
+            int  p = -1;
+            
+            pstr = clicon_option_str(h, "CLICON_NOALPN_DEFAULT");
+            if (pstr)
+                p = restconf_str2proto(pstr);
+            if (pstr == NULL || p == -1){
+                clicon_log(LOG_INFO, "%s Warning: ALPN: No protocol selected, or no ALPN?", __FUNCTION__);
+                if (restconf_close_ssl_socket(rc, __FUNCTION__, 0) < 0)
+                    goto done;
+                goto fail;
+            }
+            *proto = p;
+#else
+            *proto = HTTP_11;   /* Only http/1 */
+#endif
+#else
+            *proto = HTTP_2;    /* Only http/1 */
+#endif
+        }
+    }
+    retval = 1;
  done:
     clicon_debug(1, "%s retval:%d", __FUNCTION__, retval);
     if (cberr)
         cbuf_free(cberr);
     return retval;
+ fail:
+    retval = 0; /* ALPN not OK */
+    goto done;
 } /* ssl_alpn_check */
 
 /*! Accept new socket client. Note SSL not ip, this applies also to callhome
@@ -1316,8 +1341,9 @@ restconf_ssl_accept_client(clicon_handle    h,
         }
         if ((ret = ssl_alpn_check(h, alpn, alpnlen, rc, &proto)) < 0)
             goto done;
-        if (ret == 0)
+        if (ret == 0){
             goto closed;
+        }
         clicon_debug(1, "%s proto:%s", __FUNCTION__, restconf_proto2str(proto));
 
 #if 0 /* Seems too early to fail here, instead let authentication callback deal with this */

test/test_restconf_noalpn.sh

@@ -0,0 +1,230 @@
+#!/usr/bin/env bash
+# Restconf TLS no alpn functionality
+# Test of CLICON_RESTCONF_NOALPN_DEFAULT AND client certs
+# Also client certs (reason is usecase was POSTMAN w client certs)
+
+dir=/tmp/test_restconf_noalpn.sh
+# Magic line must be first in script (see README.md)
+s="$_" ; . ./lib.sh || if [ "$s" = $0 ]; then exit 0; else return 0; fi
+
+# Only works with native and https
+if [ "${WITH_RESTCONF}" != "native" ]; then
+    rm -rf $dir
+    if [ "$s" = $0 ]; then exit 0; else return 0; fi # skip
+fi
+
+# Only works with both http/1 and http/2
+if [ ${HAVE_LIBNGHTTP2} = false -o ${HAVE_HTTP1} = false ]; then    
+    rm -rf $dir
+    if [ "$s" = $0 ]; then exit 0; else return 0; fi # skip
+fi
+
+APPNAME=example
+
+cfg=$dir/conf.xml
+fyang=$dir/restconf.yang
+
+# Define default restconfig config: RESTCONFIG
+# RESTCONFIG=$(restconf_config none false)
+
+# Local for test here
+certdir=$dir/certs
+test -d $certdir || mkdir $certdir
+
+cakey=$certdir/ca_key.pem
+cacert=$certdir/ca_cert.pem
+srvkey=$certdir/srv_key.pem
+srvcert=$certdir/srv_cert.pem
+
+echo "cakey:$cakey"
+echo "srvkey:$srvkey"
+echo "srvcert:$srvcert"
+
+# Create server certs
+cacerts $cakey $cacert
+servercerts $cakey $cacert $srvkey $srvcert
+
+name="andy"
+
+cat<<EOF > $dir/$name.cnf
+[req]
+prompt = no
+distinguished_name = dn
+[dn]
+CN = $name # This can be verified using SSL_set1_host
+emailAddress = $name@foo.bar
+O = Clixon
+L = Stockholm
+C = SE
+EOF
+
+# Create client key
+openssl genpkey -algorithm RSA -out "$certdir/$name.key" ||  err "Generate client key"
+
+# Generate CSR (signing request)
+openssl req -new -config $dir/$name.cnf -key $certdir/$name.key -out $certdir/$name.csr
+
+# Sign by CA
+openssl x509 -req -extfile $dir/$name.cnf -days 1 -passin "pass:password" -in $certdir/$name.csr -CA $cacert -CAkey $cakey -CAcreateserial -out $certdir/$name.crt  ||  err "Generate signing client cert"
+
+cat <<EOF > $cfg
+<clixon-config xmlns="http://clicon.org/config">
+  <CLICON_FEATURE>clixon-restconf:allow-auth-none</CLICON_FEATURE> <!-- Use auth-type=none -->
+  <CLICON_CONFIGFILE>$cfg</CLICON_CONFIGFILE>
+  <CLICON_YANG_DIR>${YANG_INSTALLDIR}</CLICON_YANG_DIR>
+  <CLICON_YANG_MAIN_FILE>$fyang</CLICON_YANG_MAIN_FILE>
+  <CLICON_SOCK>/usr/local/var/$APPNAME/$APPNAME.sock</CLICON_SOCK>
+  <CLICON_BACKEND_PIDFILE>$dir/restconf.pidfile</CLICON_BACKEND_PIDFILE>
+  <CLICON_XMLDB_DIR>/usr/local/var/$APPNAME</CLICON_XMLDB_DIR>
+  <restconf>
+     <enable>true</enable>
+     <auth-type>client-certificate</auth-type>
+     <server-cert-path>$srvcert</server-cert-path>
+     <server-key-path>$srvkey</server-key-path>
+     <server-ca-cert-path>$cacert</server-ca-cert-path>
+     <debug>$DBG</debug>
+     <pretty>false</pretty>
+     <socket>
+        <namespace>default</namespace>
+        <address>0.0.0.0</address>
+        <port>443</port>
+        <ssl>true</ssl>
+     </socket>
+  </restconf>
+</clixon-config>
+EOF
+
+cat <<EOF > $fyang
+module example{
+   yang-version 1.1;
+   namespace "urn:example:clixon";
+   prefix ex;
+   container interfaces{
+      list interface{
+        key name;
+        leaf name{
+          type string;
+        }
+        leaf type{
+          mandatory true;
+          type string;
+        }
+      }
+   }
+}
+EOF
+
+new "test params: -f $cfg"
+
+if [ $BE -ne 0 ]; then
+    new "kill old backend"
+    sudo clixon_backend -zf $cfg
+    if [ $? -ne 0 ]; then
+        err
+    fi
+    sudo pkill -f clixon_backend # to be sure
+
+    new "start backend -s init -f $cfg"
+    start_backend -s init -f $cfg
+fi
+
+new "wait backend"
+wait_backend
+
+if false; then
+new "netconf POST initial tree"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><edit-config><default-operation>merge</default-operation><target><candidate/></target><config><interfaces xmlns=\"urn:example:clixon\"><interface><name>local0</name><type>regular</type></interface></interfaces></config></edit-config></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
+
+new "netconf commit"
+expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "<rpc $DEFAULTNS><commit/></rpc>" "" "<rpc-reply $DEFAULTNS><ok/></rpc-reply>"
+
+
+
+if [ $RC -ne 0 ]; then
+    new "kill old restconf daemon"
+    stop_restconf_pre
+
+    new "start restconf daemon"
+    start_restconf -f $cfg -o CLICON_NOALPN_DEFAULT=
+fi
+
+new "wait restconf"
+wait_restconf
+
+new "restconf GET http1 no-alpn expect reset"
+expectpart "$(curl $CURLOPTS --http1.1 --no-alpn --key $certdir/andy.key --cert $certdir/andy.crt -X GET $RCPROTO://localhost/restconf/data/example:interfaces/interface=local0)" "16 52 55 56"
+
+new "restconf GET http2 no-alpn expect reset"
+expectpart "$(curl $CURLOPTS --http2 --no-alpn --key $certdir/andy.key --cert $certdir/andy.crt -X GET $RCPROTO://localhost/restconf/data/example:interfaces/interface=local0)" "16 52 55 56"
+
+new "restconf GET http2 no-alpn expect reset"
+expectpart "$(curl $CURLOPTS --http2-prior-knowledge --no-alpn --key $certdir/andy.key --cert $certdir/andy.crt -X GET $RCPROTO://localhost/restconf/data/example:interfaces/interface=local0)" "16 52 55 56"
+
+fi
+
+if [ $RC -ne 0 ]; then
+    new "kill old restconf daemon"
+    stop_restconf_pre
+
+    new "start restconf daemon"
+    start_restconf -f $cfg -o "CLICON_NOALPN_DEFAULT=http/1.1" -D 1 -l f/tmp/restconf.log
+fi
+
+new "wait restconf"
+wait_restconf
+
+new "restconf GET http/1 default http/1 no-alpn"
+#expectpart "$(curl $CURLOPTS --http1.1 --no-alpn --key $certdir/andy.key --cert $certdir/andy.crt -X GET $RCPROTO://localhost/restconf/data/example:interfaces/interface=local0)" 0 "HTTP/1.1 200" '{"example:interface":\[{"name":"local0","type":"regular"}\]}'
+
+new "restconf GET http/1 default http/2 no-alpn"
+#expectpart "$(curl $CURLOPTS --http2 --no-alpn --key $certdir/andy.key --cert $certdir/andy.crt -X GET $RCPROTO://localhost/restconf/data/example:interfaces/interface=local0)" 0 "HTTP/1.1 200" '{"example:interface":\[{"name":"local0","type":"regular"}\]}'
+
+# XXX This leaks memory in restconf
+new "restconf GET http/1 default http/2 prior-knowledge no-alpn, expect fail"
+expectpart "$(curl $CURLOPTS --http2-prior-knowledge --no-alpn --key $certdir/andy.key --cert $certdir/andy.crt -X GET $RCPROTO://localhost/restconf/data/example:interfaces/interface=local0)" "16 52 55 56"
+
+if false; then
+
+if [ $RC -ne 0 ]; then
+    new "kill old restconf daemon"
+    stop_restconf_pre
+
+    new "start restconf daemon"
+    start_restconf -f $cfg -o "CLICON_NOALPN_DEFAULT=http/2"
+fi
+
+new "wait restconf"
+wait_restconf
+
+# XXX ./test_restconf_noalpn.sh: line 193: warning: command substitution: ignored null byte in input
+new "restconf GET http1 default http2 no-alpn expect fail"
+expectpart "$(curl $CURLOPTS --http1.1 --no-alpn --key $certdir/andy.key --cert $certdir/andy.crt -X GET $RCPROTO://localhost/restconf/data/example:interfaces/interface=local0 > /dev/null)" 0 --not-- HTTP
+
+new "restconf GET http2 default http2 no-alpn expect fail"
+expectpart "$(curl $CURLOPTS --http2 --no-alpn --key $certdir/andy.key --cert $certdir/andy.crt -X GET $RCPROTO://localhost/restconf/data/example:interfaces/interface=local0 2> /dev/null)" 0 --not-- HTTP
+
+new "restconf GET http2 default http2 no-alpn expect fail"
+expectpart "$(curl $CURLOPTS --http2-prior-knowledge --no-alpn --key $certdir/andy.key --cert $certdir/andy.crt -X GET $RCPROTO://localhost/restconf/data/example:interfaces/interface=local0)" 0 "HTTP/2 200" '{"example:interface":\[{"name":"local0","type":"regular"}\]}'
+
+fi
+
+if [ $RC -ne 0 ]; then
+    new "Kill restconf daemon"
+    stop_restconf 
+fi
+
+if [ $BE -ne 0 ]; then
+    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
+fi
+
+rm -rf $dir
+
+new "endtest"
+endtest

yang/clixon/Makefile.in

@@ -42,7 +42,7 @@ datarootdir     = @datarootdir@
 YANG_INSTALLDIR   = @YANG_INSTALLDIR@
 
 # Note: mirror these to test/config.sh.in
-YANGSPECS	 = clixon-config@2022-12-01.yang   # 6.1
+YANGSPECS	 = clixon-config@2023-03-01.yang   # 6.2
 YANGSPECS	+= clixon-lib@2022-12-01.yang      # 6.1
 YANGSPECS	+= clixon-rfc5277@2008-07-01.yang
 YANGSPECS	+= clixon-xml-changelog@2019-03-21.yang

yang/clixon/clixon-config@2023-03-01.yang

@@ -46,6 +46,20 @@ module clixon-config {
 
        ***** END LICENSE BLOCK *****";
 
+    revision 2023-03-01 {
+        description
+            "Added options:
+                    CLICON_RESTCONF_NOALPN_DEFAULT
+             Released in Clixon 6.2";
+    }
+    revision 2022-12-01 {
+        description
+            "Added options:
+                    CLICON_YANG_SCHEMA_MOUNT
+             Removed (previosly marked) obsolete options:
+                    CLICON_MODULE_LIBRARY_RFC7895
+             Released in Clixon 6.1";
+    }
     revision 2022-11-01 {
         description
             "Added option:
@@ -496,6 +510,12 @@ module clixon-config {
                 "Location of backend .so plugins. Load all .so 
                  plugins in this dir as backend plugins";
         }
+        leaf CLICON_YANG_SCHEMA_MOUNT{
+            type boolean;
+            description
+                "YANG schema mount, RFC 8528";
+            default false;
+        }
         leaf CLICON_BACKEND_REGEXP {
             type string;
             description
@@ -619,6 +639,21 @@ module clixon-config {
                  Note this also disables plain http/2 in prior-knowledge, that is, in http/2-only mode.
                  HTTP/2 in https(TLS) is unaffected";
         }
+        leaf CLICON_NOALPN_DEFAULT {
+            type string;
+            description
+                "By default Clixon Restconf over TLS/HTTPS uses ALPN for protocol selection.
+                 This option controls the behavior if a client does NOT use ALPN for TLS.
+                 AND both http/1 and http/2 is configured in Clixon.
+                 If the value is not set (or other value), Clixon closes the socket(reset)
+                 If the value is 'http/1.1' then HTTP/1.1 is selected
+                 If the value is 'http/2' then HTTP/2 is selected
+                 Note that if Clixon is configured for only HTTP/1 (--disable-nghttp2), 
+                 then HTTP/1 is selected if the client does not use ALPN.
+                 Likewise, if Clixon is configured for only HTTP/2 (--disable-http1), 
+                 then HTTP/2 is selected if the client does not use ALPN.
+                 This option does not apply for plain (non-TLS) HTTP";
+        }
         leaf CLICON_HTTP_DATA_PATH {
             if-feature "clrc:http-data";
             default "/";
@@ -907,8 +942,7 @@ module clixon-config {
             description
                 "If set, tag datastores with RFC 8525 YANG Module Library 
                  info. When loaded at startup, a check is made if the system
-                 yang modules match.
+                 yang modules match.";
-                 See also CLICON_MODULE_LIBRARY_RFC7895";
         }
         leaf CLICON_XMLDB_UPGRADE_CHECKOLD {
             type boolean;
@@ -1053,22 +1087,11 @@ module clixon-config {
                  restconf GET.
                  The module state data is on the form:
                        <yang-library><module-set>...
-                 If CLICON_MODULE_LIBRARY_RFC7895 is set (as well), the module state uses RFC7895
                  instead where the modile state is on the form:
                        <modules-state>...
                  See also CLICON_XMLDB_MODSTATE where the module state info is used to tag datastores
                  with module information.";
         }
-        leaf CLICON_MODULE_LIBRARY_RFC7895 {
-            type boolean;
-            default false;
-            description
-                "Enable RFC 7895 YANG Module library support as state data, instead of RFC8525.
-                 Note CLICON_YANG_LIBRARY must be enabled for this to have effect.
-                 See also CLICON_YANG_LIBRARY and CLICON_MODULE_SET_ID";
-            status obsolete;
-        }
-
         leaf CLICON_MODULE_SET_ID {
             type string;
             default "0";