diff --git a/CHANGELOG.md b/CHANGELOG.md index 03a520e2..fc144603 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/apps/restconf/restconf_native.c b/apps/restconf/restconf_native.c index 2c8e05cf..c1bbf81b 100644 --- a/apps/restconf/restconf_native.c +++ b/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, "protocolmalformed-message%s", 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 done; + goto closed; } } } @@ -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 done; + goto closed; } 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; + if (restconf_close_ssl_socket(rc, __FUNCTION__, 0) < 0) + goto done; + goto fail; } 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 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 } - if (restconf_close_ssl_socket(rc, __FUNCTION__, 0) < 0) - goto done; } - retval = 0; /* ALPN not OK */ + 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 @@ -1310,14 +1335,15 @@ restconf_ssl_accept_client(clicon_handle h, #ifndef OPENSSL_NO_NEXTPROTONEG SSL_get0_next_proto_negotiated(rc->rc_ssl, &alpn, &alpnlen); #endif /* !OPENSSL_NO_NEXTPROTONEG */ - if (alpn == NULL) { + if (alpn == NULL){ /* Returns a pointer to the selected protocol in data with length len. */ SSL_get0_alpn_selected(rc->rc_ssl, &alpn, &alpnlen); } 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 */ diff --git a/lib/src/clixon_stream.c b/lib/src/clixon_stream.c index 97a58a6e..1c36fc27 100644 --- a/lib/src/clixon_stream.c +++ b/lib/src/clixon_stream.c @@ -546,15 +546,15 @@ stream_notify(clicon_handle h, char *stream, const char *event, ...) { - int retval = -1; - va_list args; - int len; - cxobj *xev = NULL; - yang_stmt *yspec = NULL; - char *str = NULL; - cbuf *cb = NULL; - char timestr[28]; - struct timeval tv; + int retval = -1; + va_list args; + int len; + cxobj *xev = NULL; + yang_stmt *yspec = NULL; + char *str = NULL; + cbuf *cb = NULL; + char timestr[28]; + struct timeval tv; event_stream_t *es; clicon_debug(CLIXON_DBG_DETAIL, "%s", __FUNCTION__); diff --git a/test/test_restconf_noalpn.sh b/test/test_restconf_noalpn.sh new file mode 100755 index 00000000..d413d751 --- /dev/null +++ b/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< $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 < $cfg + + clixon-restconf:allow-auth-none + $cfg + ${YANG_INSTALLDIR} + $fyang + /usr/local/var/$APPNAME/$APPNAME.sock + $dir/restconf.pidfile + /usr/local/var/$APPNAME + + true + client-certificate + $srvcert + $srvkey + $cacert + $DBG + false + + default +
0.0.0.0
+ 443 + true + + + +EOF + +cat < $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" "mergelocal0regular" "" "" + +new "netconf commit" +expecteof_netconf "$clixon_netconf -qf $cfg" 0 "$DEFAULTHELLO" "" "" "" + + + +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 diff --git a/yang/clixon/Makefile.in b/yang/clixon/Makefile.in index a709524f..0d5fb336 100644 --- a/yang/clixon/Makefile.in +++ b/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 diff --git a/yang/clixon/clixon-config@2022-11-01.yang b/yang/clixon/clixon-config@2023-03-01.yang similarity index 97% rename from yang/clixon/clixon-config@2022-11-01.yang rename to yang/clixon/clixon-config@2023-03-01.yang index 42f7f469..1cca5dc6 100644 --- a/yang/clixon/clixon-config@2022-11-01.yang +++ b/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. - See also CLICON_MODULE_LIBRARY_RFC7895"; + yang modules match."; } leaf CLICON_XMLDB_UPGRADE_CHECKOLD { type boolean; @@ -1053,22 +1087,11 @@ module clixon-config { restconf GET. The module state data is on the form: ... - If CLICON_MODULE_LIBRARY_RFC7895 is set (as well), the module state uses RFC7895 instead where the modile state is on the form: ... 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";