From ef4d082f4b15cfc2c4ade5368db5358d44d0bca9 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Thu, 26 Aug 2021 16:58:52 +0200 Subject: [PATCH] * Restconf native HTTP/2: * Added option `CLICON_RESTCONF_HTTP2_PLAIN` * if disabled non-tls HTTP/2 is disabled: both direct and upgrade --- CHANGELOG.md | 4 + apps/restconf/restconf_evhtp.c | 3 +- apps/restconf/restconf_main_native.c | 7 + apps/restconf/restconf_nghttp2.c | 21 ++- test/test_restconf_http_upgrade.sh | 197 ++++++++++++++++++++++ yang/clixon/clixon-config@2021-07-11.yang | 13 ++ 6 files changed, 240 insertions(+), 5 deletions(-) create mode 100755 test/test_restconf_http_upgrade.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 708d7616..a1f47b26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,11 +60,15 @@ Users may have to change how they access the system * Native Restconf is now default, not fcgi/nginx * That is, to configure with fcgi, you need to explicitly configure: `--with-restconf=fcgi` * New clixon-config@2021-07-11.yang revision + * Added: `CLICON_RESTCONF_HTTP2_PLAIN` * Removed default of `CLICON_RESTCONF_INSTALLDIR` * The default behaviour is changed to use the config $(sbindir) to locate `clixon_restconf` when starting restconf internally ### Minor features +* Restconf native HTTP/2: + * Added option `CLICON_RESTCONF_HTTP2_PLAIN` + * if disabled non-tls HTTP/2 is disabled: both direct and upgrade * Restconf internal start: fail early if clixon_restconf binary is not found * If CLICON_BACKEND_RESTCONF_PROCESS is true * Added linenumbers to all YANG symbols for better debug and errors diff --git a/apps/restconf/restconf_evhtp.c b/apps/restconf/restconf_evhtp.c index 44983de8..0c3a4fdb 100644 --- a/apps/restconf/restconf_evhtp.c +++ b/apps/restconf/restconf_evhtp.c @@ -430,7 +430,8 @@ evhtp_upgrade_http2(clicon_handle h, char *settings; cxobj *xerr = NULL; - if ((str = restconf_param_get(h, "HTTP_UPGRADE")) != NULL){ + if ((str = restconf_param_get(h, "HTTP_UPGRADE")) != NULL && + clicon_option_bool(h, "CLICON_RESTCONF_HTTP2_PLAIN") == 1){ /* Only accept "h2c" */ if (strcmp(str, "h2c") != 0){ if (netconf_invalid_value_xml(&xerr, "protocol", "Invalid upgrade token") < 0) diff --git a/apps/restconf/restconf_main_native.c b/apps/restconf/restconf_main_native.c index a29d2373..68ebb5cf 100644 --- a/apps/restconf/restconf_main_native.c +++ b/apps/restconf/restconf_main_native.c @@ -404,6 +404,12 @@ restconf_verify_certs(int preverify_ok, case X509_V_ERR_HOSTNAME_MISMATCH: clicon_debug(1, "%s X509_V_ERR_HOSTNAME_MISMATCH", __FUNCTION__); break; + case X509_V_ERR_CERT_HAS_EXPIRED: + clicon_debug(1, "%s X509_V_ERR_CERT_HAS_EXPIRED", __FUNCTION__); + break; + case X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT: + clicon_debug(1, "%s X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT", __FUNCTION__); + break; } /* Catch a too long certificate chain. should be +1 in SSL_CTX_set_verify_depth() */ if (depth > VERIFY_DEPTH + 1) { @@ -848,6 +854,7 @@ restconf_connection(int s, #ifdef HAVE_LIBNGHTTP2 if (sd->sd_upgrade2){ nghttp2_error ngerr; + /* Switch to http/2 according to RFC 7540 Sec 3.2 and RFC 7230 Sec 6.7 */ rc->rc_proto = HTTP_2; if (http2_session_init(rc) < 0){ diff --git a/apps/restconf/restconf_nghttp2.c b/apps/restconf/restconf_nghttp2.c index 4d23e67f..797026bb 100644 --- a/apps/restconf/restconf_nghttp2.c +++ b/apps/restconf/restconf_nghttp2.c @@ -305,10 +305,22 @@ restconf_nghttp2_path(restconf_stream_data *sd) clicon_err(OE_RESTCONF, EINVAL, "arg is NULL"); goto done; } - /* Slightly awkward way of taking SSL cert subject and CN and add it to restconf parameters - * instead of accessing it directly */ - if (rc->rc_ssl != NULL){ - /* SSL subject fields, eg CN (Common Name) , can add more here? */ + + if (rc->rc_ssl == NULL){ + if (clicon_option_bool(h, "CLICON_RESTCONF_HTTP2_PLAIN") == 0){ + cxobj *xerr = NULL; + + if (netconf_operation_not_supported_xml(&xerr, "protocol", "HTTP/2 plain / non-tls is not allowed") < 0) + goto done; + if (api_return_err0(h, sd, xerr, 1, YANG_DATA_JSON, 0) < 0) + goto done; + goto ok; + } + } + else { + /* Slightly awkward way of taking SSL cert subject and CN and add it to restconf parameters + * instead of accessing it directly + * SSL subject fields, eg CN (Common Name) , can add more here? */ if (ssl_x509_name_oneline(rc->rc_ssl, &oneline) < 0) goto done; if (oneline != NULL) { @@ -327,6 +339,7 @@ restconf_nghttp2_path(restconf_stream_data *sd) } else if (api_root_restconf(h, sd, sd->sd_qvec) < 0) goto done; + ok: /* Clear (fcgi) paramaters from this request */ if (restconf_param_del_all(h) < 0) goto done; diff --git a/test/test_restconf_http_upgrade.sh b/test/test_restconf_http_upgrade.sh new file mode 100755 index 00000000..7a3581b9 --- /dev/null +++ b/test/test_restconf_http_upgrade.sh @@ -0,0 +1,197 @@ +#!/usr/bin/env bash +# +# Special test for plain(non-tls) upgrade of http/1 to http/2 +# Only native +# Three cases controlled by compile-time #ifdef:s +# http/1-only +# http/2-only +# http/1 + http/2 + +# 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 +if [ "${WITH_RESTCONF}" != "native" ]; then + if [ "$s" = $0 ]; then exit 0; else return 0; fi # skip +fi + +APPNAME=example + +cfg=$dir/conf.xml + +RCPROTO=http +RESTCONFIG=$(restconf_config none false) + +# Clixon config +cat < $cfg + + $cfg + clixon-restconf:allow-auth-none + /usr/local/share/clixon + $IETFRFC + $dir + /usr/local/lib/$APPNAME/clispec + /usr/local/lib/$APPNAME/backend + example_backend.so$ + /usr/local/lib/$APPNAME/restconf + /usr/local/lib/$APPNAME/cli + $APPNAME + /usr/local/var/$APPNAME/$APPNAME.sock + /usr/local/var/$APPNAME/$APPNAME.pidfile + /usr/local/var/$APPNAME + true + false + true + $RESTCONFIG + +EOF + + +# Restconf test routine with arguments: +# 1. Enable http for http/2 (CLICON_RESTCONF_HTTP2_PLAIN) +# 2. Expected http return value +function testrun() +{ + h2enable=$1 + + new "test params: -f $cfg -- -s" + 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 [ $RC -ne 0 ]; then + new "kill old restconf daemon" + stop_restconf_pre + + new "start restconf daemon -o CLICON_RESTCONF_HTTP2_PLAIN=${h2enable}" + start_restconf -f $cfg -o CLICON_RESTCONF_HTTP2_PLAIN=${h2enable} + fi + + + + if [ ${HAVE_LIBNGHTTP2} = false -a ${HAVE_LIBEVHTP} = true ]; then # http/1 only + + new "wait restconf" + wait_restconf + + # http/1-only always stays on http/1 in http/1 + http/2 mode + new "restconf http1.1 no upgrade (h2:$h2enable)" + echo "curl -Ssik --http1.1 -X GET http://localhost/.well-known/host-meta" + expectpart "$(curl -Ssik --http1.1 -X GET http://localhost/.well-known/host-meta)" 0 "HTTP/1.1 200" "" "" "" --not-- "HTTP/2" + + # http/1->http/2 switched if h2enable, otherwise it stays in http/1 + new "restconf upgrade http1->http2 (h2:$h2enable)" + echo "curl -Ssik --http2 -X GET http://localhost/.well-known/host-meta" + # stay on http/1 + expectpart "$(curl -Ssik --http2 -X GET http://localhost/.well-known/host-meta)" 0 "HTTP/1.1 200" "" "" "" --not-- "HTTP/2" + + # http/2-only is always an error in http/1 + http/2 mode + new "restconf http2 prior-knowledge (h2:$h2enable)" + echo "curl -Ssik --http2-prior-knowledge -X GET http://localhost/.well-known/host-meta" + expectpart "$(curl -Ssik --http2-prior-knowledge -X GET http://localhost/.well-known/host-meta 2>&1)" "16 55" + + elif [ ${HAVE_LIBNGHTTP2} = true -a ${HAVE_LIBEVHTP} = false ]; then # http/2 only + + sleep 2 # Cannot do wait restconf + + # http/1-only always stays on http/1 in http/1 + http/2 mode + new "restconf http1.1 no upgrade (h2:$h2enable)" + echo "curl -Ssik --http1.1 -X GET http://localhost/.well-known/host-meta" + # XXX cannot use expectpart due to null in pipe + curl -Ssik --http1.1 -X GET http://localhost/.well-known/host-meta + if [ $? == 0 ]; then + err "NULL" "sucess" + fi + + # http/1->http/2 switched if h2enable, otherwise it stays in http/1 + new "restconf upgrade http1->http2 (h2:$h2enable)" + echo "curl -Ssik --http2 -X GET http://localhost/.well-known/host-meta" + # stay on http/1 + curl -Ssik --http2 -X GET http://localhost/.well-known/host-meta + if [ $? == 0 ]; then + err "NULL" "sucess" + fi + + # http/2-only is always an error in http/1 + http/2 mode + new "restconf http2 prior-knowledge (h2:$h2enable)" + echo "curl -Ssik --http2-prior-knowledge -X GET http://localhost/.well-known/host-meta" + if $h2enable; then + new "wait restconf" + wait_restconf + + expectpart "$(curl -Ssik --http2-prior-knowledge -X GET http://localhost/.well-known/host-meta 2>&1)" 0 "HTTP/2 200" "" "" "" --not-- "HTTP/1.1" + + else + expectpart "$(curl -Ssik --http2-prior-knowledge -X GET http://localhost/.well-known/host-meta 2>&1)" 0 "HTTP/2 405" + fi + + elif [ ${HAVE_LIBNGHTTP2} = true -a ${HAVE_LIBEVHTP} = true ]; then # http/1 + http/2 + + new "wait restconf" + wait_restconf + + # http/1-only always stays on http/1 in http/1 + http/2 mode + new "restconf http1.1 no upgrade (h2:$h2enable)" + echo "curl -Ssik --http1.1 -X GET http://localhost/.well-known/host-meta" + expectpart "$(curl -Ssik --http1.1 -X GET http://localhost/.well-known/host-meta)" 0 "HTTP/1.1 200" "" "" "" --not-- "HTTP/2" + + # http/1->http/2 switched if h2enable, otherwise it stays in http/1 + new "restconf upgrade http1->http2 (h2:$h2enable)" + echo "curl -Ssik --http2 -X GET http://localhost/.well-known/host-meta" + if $h2enable; then + # switch to http/2 + expectpart "$(curl -Ssik --http2 -X GET http://localhost/.well-known/host-meta)" 0 "HTTP/2 200" "" "" "" "HTTP/1.1 101 Switching" "HTTP/2 200" --not-- "HTTP/1.1 200" + else + # stay on http/1 + expectpart "$(curl -Ssik --http2 -X GET http://localhost/.well-known/host-meta)" 0 "HTTP/1.1 200" "" "" "" --not-- "HTTP/2" + fi + + # http/2-only is always an error in http/1 + http/2 mode + new "restconf http2 prior-knowledge (h2:$h2enable)" + echo "curl -Ssik --http2-prior-knowledge -X GET http://localhost/.well-known/host-meta" + expectpart "$(curl -Ssik --http2-prior-knowledge -X GET http://localhost/.well-known/host-meta 2>&1)" "16 55" + + 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 +} + +new "disable plain http/2" +testrun false + +new "enable plain http/2" +testrun true + +# Set by restconf_config +unset RESTCONFIG +unset RCPROTO + +rm -rf $dir + +new "endtest" +endtest diff --git a/yang/clixon/clixon-config@2021-07-11.yang b/yang/clixon/clixon-config@2021-07-11.yang index c4dea419..bf7f34b8 100644 --- a/yang/clixon/clixon-config@2021-07-11.yang +++ b/yang/clixon/clixon-config@2021-07-11.yang @@ -47,6 +47,7 @@ module clixon-config { description "Added option: CLICON_SYSTEM_CAPABILITIES + CLICON_RESTCONF_HTTP2_PLAIN Removed default value: CLICON_RESTCONF_INSTALLDIR Marked as obsolete: @@ -583,6 +584,18 @@ module clixon-config { must be set to 'none'. "; } + leaf CLICON_RESTCONF_HTTP2_PLAIN { + type boolean; + default true; + description + "Applies to plan (non-tls) http/2 ie when clixon is configured with --enable-nghttp2 + If false, disable direct and upgrade for plain(non-tls) HTTP/2. + If true, allows direct and upgrade for plain(non-tls) HTTP/2. + This is especially useful in http/1 + http/2 mode to avoid the complex upgrade/switch + from http/1 to http/2. + 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_CLI_DIR { type string; description