Native restconf fixes:

- memleak in http/1-only
- exit if http/1 request sent to http/2-only
- hang if http/1 TLS request sent to http/2 only
This commit is contained in:
Olof hagsand 2021-07-09 15:00:47 +02:00
parent ed1d5698e8
commit 270c47b396
6 changed files with 123 additions and 38 deletions

View file

@ -42,6 +42,10 @@ Developers may need to change their code
### Corrected Bugs
* Native RESTCONF fixes for http/1 or http/2 only modes
* Memleak in http/1-only
* Exit if http/1 request sent to http/2-only (bad client magic)
* Hang if http/1 TLS request sent to http/2 only (alpn accepted http/1.1)
* Fixed: [RESTConf GET for a specific list instance retrieves data from other submodules that have same list name and key value #244](https://github.com/clicon/clixon/issues/244)
## 5.2.0

View file

@ -515,6 +515,8 @@ restconf_path_root(evhtp_request_t *req,
if (clicon_debug_get())
evhtp_headers_for_each(req->headers_in, evhtp_print_header, h);
/* Query vector, ie the ?a=x&b=y stuff */
if (sd->sd_qvec)
cvec_free(sd->sd_qvec);
if ((sd->sd_qvec = cvec_new(0)) ==NULL){
clicon_err(OE_UNIX, errno, "cvec_new");
evhtp_internal_error(req);

View file

@ -467,14 +467,16 @@ alpn_select_proto_cb(SSL *ssl,
inp++;
if (clicon_debug_get()) /* debug print the protoocol */
alpn_proto_dump(__FUNCTION__, (const char*)inp, len);
#ifdef HAVE_LIBEVHTP
if (pref < 10 && len == 8 && strncmp((char*)inp, "http/1.1", len) == 0){
*outlen = len;
*out = inp;
pref = 10;
}
#endif
#ifdef HAVE_LIBNGHTTP2
/* Higher pref than http/1.1 */
else if (pref < 20 && len == 2 && strncmp((char*)inp, "h2", len) == 0){
if (pref < 20 && len == 2 && strncmp((char*)inp, "h2", len) == 0){
*outlen = len;
*out = inp;
pref = 20;
@ -684,7 +686,9 @@ restconf_connection(int s,
char buf[BUFSIZ]; /* from stdio.h, typically 8K XXX: reduce for test */
int readmore = 1;
int sslerr;
#ifdef HAVE_LIBNGHTTP2
int ret;
#endif
#ifdef HAVE_LIBEVHTP
clicon_handle h;
evhtp_connection_t *evconn = NULL;
@ -889,9 +893,14 @@ restconf_connection(int s,
#endif /* HAVE_LIBEVHTP */
#ifdef HAVE_LIBNGHTTP2
case HTTP_2:
if (http2_recv(rc, (unsigned char *)buf, n) < 0)
if ((ret = http2_recv(rc, (unsigned char *)buf, n)) < 0)
goto done;
//notused sd = restconf_stream_find(rc, 0); /* default stream */
if (ret == 0){
restconf_close_ssl_socket(rc, 1);
if (restconf_conn_free(rc) < 0)
goto done;
goto ok;
}
/* There may be more data frames */
readmore++;
break;
@ -1011,7 +1020,7 @@ ssl_alpn_check(clicon_handle h,
}
if (alpn != NULL){
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>ALPN: protocol not recognized: %s</error-message></error></errors>", alpn);
clicon_log(LOG_NOTICE, "Warning: %s", cbuf_get(cberr));
clicon_log(LOG_INFO, "%s Warning: %s", __FUNCTION__, cbuf_get(cberr));
if (send_badrequest(h, rc->rc_s, rc->rc_ssl,
"application/yang-data+xml",
cbuf_get(cberr)) < 0)
@ -1019,7 +1028,7 @@ ssl_alpn_check(clicon_handle h,
}
else{
/* XXX Sending badrequest here gives a segv in SSL_shutdown() later or a SIGPIPE here */
clicon_log(LOG_NOTICE, "Warning: ALPN: No protocol selected");
clicon_log(LOG_INFO, "%s Warning: ALPN: No protocol selected", __FUNCTION__);
}
if (rc->rc_ssl){
@ -1029,7 +1038,7 @@ ssl_alpn_check(clicon_handle h,
if ((ret = SSL_shutdown(rc->rc_ssl)) < 0){
int e = SSL_get_error(rc->rc_ssl, ret);
if (e == SSL_ERROR_SYSCALL){
clicon_log(LOG_NOTICE, "Warning: SSL_shutdown SSL_ERROR_SYSCALL");
clicon_log(LOG_INFO, "%s Warning: SSL_shutdown SSL_ERROR_SYSCALL", __FUNCTION__);
/* Continue */
}
else {
@ -1577,7 +1586,7 @@ restconf_openssl_init(clicon_handle h,
}
int status = setrlimit(RLIMIT_CORE, &rlp);
if (status != 0) {
clicon_log(LOG_NOTICE, "%s: setrlimit() failed, %s", __func__, strerror(errno));
clicon_log(LOG_INFO, "%s: setrlimit() failed, %s", __FUNCTION__, strerror(errno));
}
}

View file

@ -865,8 +865,14 @@ error_callback2(nghttp2_session *session,
}
#endif
/*
* XXX see session_recv
/*! Process an HTTP/2 request received in buffer, process request and send reply
*
* @param[in] rc Restconf connection
* @param[in] buf Character buffer
* @param[in] n Lenght of buf
* @retval 1 OK
* @retval 0 Invald request
* @retval -1 Fatal error
*/
int
http2_recv(restconf_conn *rc,
@ -884,6 +890,18 @@ http2_recv(restconf_conn *rc,
}
/* may make additional pending frames */
if ((ngerr = nghttp2_session_mem_recv(rc->rc_ngsession, buf, n)) < 0){
if (ngerr == NGHTTP2_ERR_BAD_CLIENT_MAGIC){
/* :enum:`NGHTTP2_ERR_BAD_CLIENT_MAGIC`
* Invalid client magic was detected. This error only returns
* when |session| was configured as server and
* `nghttp2_option_set_no_recv_client_magic()` is not used with
* nonzero value. */
clicon_log(LOG_INFO, "%s Received bad client magic byte strin", __FUNCTION__);
/* unsure if this does anything, byt does not seem to hurt */
if ((ngerr = nghttp2_session_terminate_session(rc->rc_ngsession, ngerr)) < 0)
clicon_err(OE_NGHTTP2, ngerr, "nghttp2_session_terminate_session %d", ngerr);
goto fail;
}
clicon_err(OE_NGHTTP2, ngerr, "nghttp2_session_mem_recv");
goto done;
}
@ -895,9 +913,13 @@ http2_recv(restconf_conn *rc,
clicon_err(OE_NGHTTP2, ngerr, "nghttp2_session_send");
goto done;
}
retval = 0;
retval = 1; /* OK */
done:
clicon_debug(1, "%s retval:%d", __FUNCTION__, retval);
return retval;
fail:
retval = 0;
goto done;
}
/* Send HTTP/2 client connection header, which includes 24 bytes

View file

@ -172,37 +172,86 @@ function testrun()
echo "curl $CURLOPTS -X GET $proto://$addr/.well-known/host-meta"
expectpart "$(curl $CURLOPTS -X GET $proto://$addr/.well-known/host-meta)" 0 "HTTP/$HVER 200" "<XRD xmlns='http://docs.oasis-open.org/ns/xri/xrd-1.0'>" "<Link rel='restconf' href='/restconf'/>" "</XRD>"
if ! ${HAVE_LIBNGHTTP2}; then # http/2
if [ ${HAVE_LIBNGHTTP2} = true -a ${HAVE_LIBEVHTP} = false ]; then
echo "native + http/2 only"
# Important here is robustness of restconf daemon, not a meaningful reply
if [ $proto = http ]; then # see (2) https to http port in restconf_main_native.c
# http protocol mismatch can just close the socket if assumed its http/2
# everything else would guess it is http/1 which is really wrong here
# The tr statement replaces null char to get rid of annoying message:
# ./test_restconf.sh: line 180: warning: command substitution: ignored null byte in input
new "restconf GET http/1.0 - close"
expectpart "$(curl $CURLOPTS --http1.0 -X GET $proto://$addr/.well-known/host-meta | tr '\0' '\n')" 0 "" --not-- 'HTTP'
else
new "restconf GET https/1.0 - close"
expectpart "$(curl $CURLOPTS --http1.0 -X GET $proto://$addr/.well-known/host-meta)" 52 "" --not-- 'HTTP'
if [ "${WITH_RESTCONF}" = "native" ]; then # XXX does not work with nginx
new "restconf GET http/1.0 - returns 1.0"
expectpart "$(curl $CURLOPTS --http1.0 -X GET $proto://$addr/.well-known/host-meta)" 0 'HTTP/1.0 200 OK' "<XRD xmlns='http://docs.oasis-open.org/ns/xri/xrd-1.0'>" "<Link rel='restconf' href='/restconf'/>" "</XRD>"
fi
new "restconf GET http/1.1"
expectpart "$(curl $CURLOPTS --http1.1 -X GET $proto://$addr/.well-known/host-meta)" 0 'HTTP/1.1 200 OK' "<XRD xmlns='http://docs.oasis-open.org/ns/xri/xrd-1.0'>" "<Link rel='restconf' href='/restconf'/>" "</XRD>"
fi
# Try http/2 - go back to http/1.1
new "restconf GET http/2"
expectpart "$(curl $CURLOPTS --http2 -X GET $proto://$addr/.well-known/host-meta)" 0 "HTTP/1.1 200 OK" "<XRD xmlns='http://docs.oasis-open.org/ns/xri/xrd-1.0'>" "<Link rel='restconf' href='/restconf'/>" "</XRD>"
if [ $proto = http ]; then
new "restconf GET http/1.1 - close"
expectpart "$(curl $CURLOPTS --http1.1 -X GET $proto://$addr/.well-known/host-meta | tr '\0' '\n')" 0 --not-- 'HTTP'
else
new "restconf GET https/1.1 - close"
expectpart "$(curl $CURLOPTS --http1.1 -X GET $proto://$addr/.well-known/host-meta)" 52 --not-- 'HTTP'
fi
if [ $proto = http ]; then
new "restconf GET http/2 switch protocol"
expectpart "$(curl $CURLOPTS --http2 -X GET $proto://$addr/.well-known/host-meta | tr '\0' '\n')" 0 --not-- 'HTTP'
else
new "restconf GET https/2 alpn protocol"
expectpart "$(curl $CURLOPTS --http2 -X GET $proto://$addr/.well-known/host-meta)" 0 "HTTP/$HVER 200" "<XRD xmlns='http://docs.oasis-open.org/ns/xri/xrd-1.0'>" "<Link rel='restconf' href='/restconf'/>" "</XRD>"
fi
if [ $proto = http ]; then # see (2) https to http port in restconf_main_native.c
new "restconf GET http/2 prior-knowledge (http)"
expectpart "$(curl $CURLOPTS --http2-prior-knowledge -X GET $proto://$addr/.well-known/host-meta 2>&1)" "16 52 55" # "Error in the HTTP2 framing layer" "Connection reset by peer"
# Wrong protocol http when https or vice versa
if [ $proto = http ]; then # see (2) https to http port in restconf_main_native.c
new "Wrong proto=https on http port, expect err 35 wrong version number"
expectpart "$(curl $CURLOPTS -X GET https://$addr:80/.well-known/host-meta 2>&1)" 35 #"wrong version number" # dependent on curl version
else # see (1) http to https port in restconf_main_native.c
new "Wrong proto=http on https port, expect bad request"
expectpart "$(curl $CURLOPTS -X GET http://$addr:443/.well-known/host-meta)" "16 52 55" --not-- 'HTTP'
fi
else
new "restconf GET http/2 prior-knowledge(http2)"
expectpart "$(curl $CURLOPTS --http2-prior-knowledge -X GET $proto://$addr/.well-known/host-meta)" 0 'HTTP/1.1 200 OK' "<XRD xmlns='http://docs.oasis-open.org/ns/xri/xrd-1.0'>" "<Link rel='restconf' href='/restconf'/>" "</XRD>"
fi
echo "fcgi or native+http/1 or native+http/1+http/2"
if [ "${WITH_RESTCONF}" = "native" ]; then # XXX does not work with nginx
new "restconf GET http/1.0 - returns 1.0"
expectpart "$(curl $CURLOPTS --http1.0 -X GET $proto://$addr/.well-known/host-meta)" 0 'HTTP/1.0 200 OK' "<XRD xmlns='http://docs.oasis-open.org/ns/xri/xrd-1.0'>" "<Link rel='restconf' href='/restconf'/>" "</XRD>"
fi
new "restconf GET http/1.1"
expectpart "$(curl $CURLOPTS --http1.1 -X GET $proto://$addr/.well-known/host-meta)" 0 'HTTP/1.1 200 OK' "<XRD xmlns='http://docs.oasis-open.org/ns/xri/xrd-1.0'>" "<Link rel='restconf' href='/restconf'/>" "</XRD>"
# Negative test GET datastore
if [ $proto = http ]; then # see (2) https to http port in restconf_main_native.c
new "Wrong proto=https on http port, expect err 35 wrong version number"
expectpart "$(curl $CURLOPTS -X GET https://$addr:80/.well-known/host-meta 2>&1)" 35 #"wrong version number" # dependent on curl version
else # see (1) http to https port in restconf_main_native.c
new "Wrong proto=http on https port, expect bad request"
expectpart "$(curl $CURLOPTS -X GET http://$addr:443/.well-known/host-meta)" 0 "HTTP/$HVER 400"
# expectpart "$(curl $CURLOPTS -X GET http://$addr:443/.well-known/host-meta 2>&1)" 56 "Connection reset by peer"
fi
if ${HAVE_LIBNGHTTP2}; then
# http/1 + http/2
new "restconf GET http/2 switch protocol"
expectpart "$(curl $CURLOPTS --http2 -X GET $proto://$addr/.well-known/host-meta)" 0 "" "HTTP/2 200" "<XRD xmlns='http://docs.oasis-open.org/ns/xri/xrd-1.0'>" "<Link rel='restconf' href='/restconf'/>" "</XRD>" # Only if http: HTTP/1.1 101 Switching Protocols
else
# http/1 only Try http/2 - go back to http/1.1
new "restconf GET http/2 switch protocol"
expectpart "$(curl $CURLOPTS --http2 -X GET $proto://$addr/.well-known/host-meta)" 0 "HTTP/1.1 200 OK" "<XRD xmlns='http://docs.oasis-open.org/ns/xri/xrd-1.0'>" "<Link rel='restconf' href='/restconf'/>" "</XRD>"
fi
# http2-prior knowledge
if [ $proto = http ]; then # see (2) https to http port in restconf_main_native.c
new "restconf GET http/2 prior-knowledge (http)"
expectpart "$(curl $CURLOPTS --http2-prior-knowledge -X GET $proto://$addr/.well-known/host-meta 2>&1)" "16 52 55" # "Error in the HTTP2 framing layer" "Connection reset by peer"
else
new "restconf GET https/2 prior-knowledge"
expectpart "$(curl $CURLOPTS --http2-prior-knowledge -X GET $proto://$addr/.well-known/host-meta)" 0 "HTTP/$HVER 200" "<XRD xmlns='http://docs.oasis-open.org/ns/xri/xrd-1.0'>" "<Link rel='restconf' href='/restconf'/>" "</XRD>"
fi
# Wrong protocol http when https or vice versa
if [ $proto = http ]; then # see (2) https to http port in restconf_main_native.c
new "Wrong proto=https on http port, expect err 35 wrong version number"
expectpart "$(curl $CURLOPTS -X GET https://$addr:80/.well-known/host-meta 2>&1)" 35 #"wrong version number" # dependent on curl version
else # see (1) http to https port in restconf_main_native.c
new "Wrong proto=http on https port, expect bad request"
expectpart "$(curl $CURLOPTS -X GET http://$addr:443/.well-known/host-meta)" 0 "HTTP/" "400"
# expectpart "$(curl $CURLOPTS -X GET http://$addr:443/.well-known/host-meta 2>&1)" 56 "Connection reset by peer"
fi
fi # HTTP/2
# Exact match
new "restconf get restconf resource. RFC 8040 3.3 (json)"
expectpart "$(curl $CURLOPTS -X GET -H "Accept: application/yang-data+json" $proto://$addr/restconf)" 0 "HTTP/$HVER 200" '{"ietf-restconf:restconf":{"data":{},"operations":{},"yang-library-version":"2019-01-04"}}'
@ -341,7 +390,7 @@ function testrun()
new "restconf Check eth/0/0 GET augmented state level 2"
expectpart "$(curl $CURLOPTS -X GET -H 'Accept: application/yang-data+json' $proto://$addr/restconf/data/ietf-interfaces:interfaces/interface=eth%2f0%2f0/clixon-example:my-status)" 0 "HTTP/$HVER 200" '{"clixon-example:my-status":{"int":42,"str":"foo"}}'
new "restconf Check eth/0/0 added state XXXXXXX"
new "restconf Check eth/0/0 added state"
expectpart "$(curl $CURLOPTS -X GET -H 'Accept: application/yang-data+json' $proto://$addr/restconf/data/clixon-example:state)" 0 "HTTP/$HVER 200" '{"clixon-example:state":{"op":\["41","42","43"\]}}'
new "restconf Re-post eth/0/0 which should generate error"

View file

@ -204,7 +204,6 @@ new "restconf DELETE whole datastore"
expectpart "$(curl $CURLOPTS -X DELETE $RCPROTO://localhost/restconf/data)" 0 "HTTP/$HVER 204"
#--------------- Multiple request in single TCP tests
# XXX THIS HAS LEAKS IN HTTP/1 ONLY!
expectpart "$(curl $CURLOPTS -H "Accept: application/yang-data+xml" -X GET $RCPROTO://localhost/restconf/data?content=config --next $CURLOPTS -H "Content-Type: application/yang-data+json" -X POST $RCPROTO://localhost/restconf/data -d '{"example:cont1":{"interface":{"name":"local0","type":"regular"}}}')" 0 "HTTP/$HVER 200" '<data/>' "HTTP/$HVER 201"
#--------------- json type tests