From cbf8c2e019c9280eb493f8c5d26ef4d1eb830771 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 22 Oct 2021 15:58:46 -0700 Subject: [PATCH 1/5] statschannel doesn't handle multiple reads correctly if an incoming HTTP request is incomplete, but nothing else is clearly wrong with it, the stats channel continues reading to see if there's more coming. the buffer length was not being processed correctly in this case. also, the server state was not reset correctly when the request was complete, so that subsequent requests could be appended to the first buffer instead of being treated as new. in addition fixing the above problems, this commit also increases the size of the httpd request buffer from 1024 to 4096, because some browsers send a lot of headers. --- lib/isc/httpd.c | 92 +++++++++++++++++++++++++------------------------ 1 file changed, 47 insertions(+), 45 deletions(-) diff --git a/lib/isc/httpd.c b/lib/isc/httpd.c index b71d323823..c9f78ef331 100644 --- a/lib/isc/httpd.c +++ b/lib/isc/httpd.c @@ -38,7 +38,7 @@ } \ } while (0) -#define HTTP_RECVLEN 1024 +#define HTTP_RECVLEN 4096 #define HTTP_SENDGROW 1024 #define HTTP_SEND_MAXLEN 10240 @@ -117,7 +117,6 @@ struct isc_httpd { * the backing store. * The third buffer is compbuffer, managed by us, that contains the * compressed HTTP data, if compression is used. - * */ isc_buffer_t headerbuffer; isc_buffer_t compbuffer; @@ -172,7 +171,7 @@ static isc_result_t httpd_response(isc_httpd_t *); static isc_result_t -process_request(isc_httpd_t *, isc_region_t *); +process_request(isc_httpd_t *, isc_region_t *, size_t *); static isc_result_t grow_headerspace(isc_httpd_t *); @@ -196,7 +195,7 @@ free_buffer(isc_mem_t *mctx, isc_buffer_t *buffer) { isc_region_t r; isc_buffer_region(buffer, &r); - if (r.length > 0) { + if (r.base != NULL) { isc_mem_put(mctx, r.base, r.length); } } @@ -393,13 +392,20 @@ have_header(isc_httpd_t *httpd, const char *header, const char *value, } static isc_result_t -process_request(isc_httpd_t *httpd, isc_region_t *region) { - char *s = NULL, *p = NULL; +process_request(isc_httpd_t *httpd, isc_region_t *region, size_t *buflen) { + char *s = NULL, *p = NULL, *urlend = NULL; + size_t limit = sizeof(httpd->recvbuf) - httpd->recvlen - 1; + size_t len = region->length; int delim; - memmove(httpd->recvbuf + httpd->recvlen, region->base, region->length); - httpd->recvlen += region->length; + if (len > limit) { + len = limit; + } + + memmove(httpd->recvbuf + httpd->recvlen, region->base, len); + httpd->recvlen += len; httpd->recvbuf[httpd->recvlen] = 0; + *buflen = httpd->recvlen; httpd->headers = NULL; /* @@ -417,7 +423,7 @@ process_request(isc_httpd_t *httpd, isc_region_t *region) { } /* - * NUL terminate request at the blank line. + * NULL terminate the request at the blank line. */ s[delim] = 0; @@ -454,13 +460,13 @@ process_request(isc_httpd_t *httpd, isc_region_t *region) { if (!BUFLENOK(s)) { return (ISC_R_NOMEMORY); } - *s = 0; + urlend = s; /* * Make the URL relative. */ - if ((strncmp(p, "http:/", 6) == 0) || (strncmp(p, "https:/", 7) == 0)) { - /* Skip first / */ + if (strncmp(p, "http://", 7) == 0 || strncmp(p, "https://", 8) == 0) { + /* Skip first '/' */ while (*p != '/' && *p != 0) { p++; } @@ -468,7 +474,7 @@ process_request(isc_httpd_t *httpd, isc_region_t *region) { return (ISC_R_RANGE); } p++; - /* Skip second / */ + /* Skip second '/' */ while (*p != '/' && *p != 0) { p++; } @@ -476,7 +482,7 @@ process_request(isc_httpd_t *httpd, isc_region_t *region) { return (ISC_R_RANGE); } p++; - /* Find third / */ + /* Find third '/' */ while (*p != '/' && *p != 0) { p++; } @@ -491,7 +497,7 @@ process_request(isc_httpd_t *httpd, isc_region_t *region) { s = p; /* - * Now, see if there is a ? mark in the URL. If so, this is + * Now, see if there is a question mark in the URL. If so, this is * part of the query string, and we will split it from the URL. */ httpd->querystring = strchr(httpd->url, '?'); @@ -566,6 +572,16 @@ process_request(isc_httpd_t *httpd, isc_region_t *region) { return (ISC_R_RANGE); } + /* + * Looks like a a valid request, so now we know we won't have + * to process this buffer again. We can NULL-terminate the + * URL for the caller's benefit, and set recvlen to 0 so + * the next read will overwrite this one instead of appending + * to the buffer. + */ + *urlend = 0; + httpd->recvlen = 0; + return (ISC_R_SUCCESS); } @@ -596,24 +612,24 @@ httpd_reset(void *arg) { isc_buffer_clear(&httpd->headerbuffer); isc_buffer_clear(&httpd->compbuffer); isc_buffer_invalidate(&httpd->bodybuffer); - - httpdmgr_detach(&httpdmgr); } static void httpd_put(void *arg) { isc_httpd_t *httpd = (isc_httpd_t *)arg; - isc_httpdmgr_t *httpdmgr = NULL; + isc_httpdmgr_t *mgr = NULL; REQUIRE(VALID_HTTPD(httpd)); - httpdmgr = httpd->mgr; - REQUIRE(VALID_HTTPDMGR(httpdmgr)); + mgr = httpd->mgr; + REQUIRE(VALID_HTTPDMGR(mgr)); httpd->magic = 0; + httpd->mgr = NULL; - free_buffer(httpdmgr->mctx, &httpd->headerbuffer); - free_buffer(httpdmgr->mctx, &httpd->compbuffer); + free_buffer(mgr->mctx, &httpd->headerbuffer); + free_buffer(mgr->mctx, &httpd->compbuffer); + httpdmgr_detach(&mgr); #if ENABLE_AFL if (finishhook != NULL) { @@ -633,6 +649,7 @@ new_httpd(isc_httpdmgr_t *httpdmgr, isc_nmhandle_t *handle) { if (httpd == NULL) { httpd = isc_nmhandle_getextra(handle); *httpd = (isc_httpd_t){ .handle = NULL }; + httpdmgr_attach(httpdmgr, &httpd->mgr); } if (httpd->handle == NULL) { @@ -642,8 +659,6 @@ new_httpd(isc_httpdmgr_t *httpdmgr, isc_nmhandle_t *handle) { INSIST(httpd->handle == handle); } - httpdmgr_attach(httpdmgr, &httpd->mgr); - /* * Initialize the buffer for our headers. */ @@ -749,22 +764,15 @@ render_500(const char *url, isc_httpdurl_t *urlinfo, const char *querystring, /*%< * Reallocates compbuffer to size, does nothing if compbuffer is already * larger than size. - * - * Requires: - *\li httpd a valid isc_httpd_t object - * - * Returns: - *\li #ISC_R_SUCCESS -- all is well. - *\li #ISC_R_NOMEMORY -- not enough memory to extend buffer */ -static isc_result_t +static void alloc_compspace(isc_httpd_t *httpd, unsigned int size) { - char *newspace; + char *newspace = NULL; isc_region_t r; isc_buffer_region(&httpd->compbuffer, &r); if (size < r.length) { - return (ISC_R_SUCCESS); + return; } newspace = isc_mem_get(httpd->mgr->mctx, size); @@ -773,8 +781,6 @@ alloc_compspace(isc_httpd_t *httpd, unsigned int size) { if (r.base != NULL) { isc_mem_put(httpd->mgr->mctx, r.base, r.length); } - - return (ISC_R_SUCCESS); } /*%< @@ -794,15 +800,11 @@ static isc_result_t httpd_compress(isc_httpd_t *httpd) { z_stream zstr; isc_region_t r; - isc_result_t result; int ret; int inputlen; inputlen = isc_buffer_usedlength(&httpd->bodybuffer); - result = alloc_compspace(httpd, inputlen); - if (result != ISC_R_SUCCESS) { - return (result); - } + alloc_compspace(httpd, inputlen); isc_buffer_region(&httpd->compbuffer, &r); /* @@ -842,6 +844,7 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t r; bool is_compressed = false; char datebuf[ISC_FORMATHTTPTIMESTAMP_SIZE]; + size_t buflen = 0; httpd = isc_nmhandle_getdata(handle); @@ -851,10 +854,10 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, goto cleanup_readhandle; } - result = process_request(httpd, region); + result = process_request(httpd, region, &buflen); if (result == ISC_R_NOTFOUND) { - if (httpd->recvlen < HTTP_RECVLEN - 1) { - /* don't unref, continue reading */ + if (buflen < HTTP_RECVLEN - 1) { + /* don't unref, keep reading */ return; } goto cleanup_readhandle; @@ -960,7 +963,6 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, isc_nmhandle_attach(handle, &httpd->sendhandle); isc_nm_send(handle, &r, httpd_senddone, httpd); - return; cleanup_readhandle: isc_nmhandle_detach(&httpd->readhandle); } From e46c64bf423f2e7ac726217a6aa83ce7947f4040 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 26 Oct 2021 11:54:31 +1100 Subject: [PATCH 2/5] Consume the HTTP headers after processing a request Remember the amount of space consumed by the HTTP headers, then move any trailing data to the start of the httpd->recvbuf once we have finished processing the request. --- lib/isc/httpd.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/lib/isc/httpd.c b/lib/isc/httpd.c index c9f78ef331..9596b8e0af 100644 --- a/lib/isc/httpd.c +++ b/lib/isc/httpd.c @@ -87,6 +87,7 @@ struct isc_httpd { */ char recvbuf[HTTP_RECVLEN]; /*%< receive buffer */ uint32_t recvlen; /*%< length recv'd */ + uint32_t consume; /*%< length of last command */ char *headers; /*%< set in process_request() */ method_t method; char *url; @@ -417,9 +418,12 @@ process_request(isc_httpd_t *httpd, isc_region_t *region, size_t *buflen) { if (s == NULL) { s = strstr(httpd->recvbuf, "\n\n"); delim = 1; - } - if (s == NULL) { - return (ISC_R_NOTFOUND); + if (s == NULL) { + return (ISC_R_NOTFOUND); + } + httpd->consume = s + 2 - httpd->recvbuf; + } else { + httpd->consume = s + 4 - httpd->recvbuf; } /* @@ -580,7 +584,6 @@ process_request(isc_httpd_t *httpd, isc_region_t *region, size_t *buflen) { * to the buffer. */ *urlend = 0; - httpd->recvlen = 0; return (ISC_R_SUCCESS); } @@ -602,6 +605,7 @@ httpd_reset(void *arg) { httpd->recvbuf[0] = 0; httpd->recvlen = 0; + httpd->consume = 0; httpd->headers = NULL; httpd->method = METHOD_UNKNOWN; httpd->url = NULL; @@ -952,6 +956,17 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, result = isc_buffer_copyregion(httpd->sendbuffer, &r); RUNTIME_CHECK(result == ISC_R_SUCCESS); + /* Consume the request from the recv buffer. */ + if (httpd->consume != 0U) { + INSIST(httpd->consume <= httpd->recvlen); + if (httpd->consume < httpd->recvlen) { + memmove(httpd->recvbuf, httpd->recvbuf + httpd->consume, + httpd->recvlen - httpd->consume); + } + httpd->recvlen -= httpd->consume; + httpd->consume = 0; + } + /* * Determine total response size. */ From 49531e45821bbebebbd097337da906042c3e964c Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 26 Oct 2021 15:28:36 +1100 Subject: [PATCH 3/5] Handle HTTP/1.1 pipelined requests Check to see whether there are outstanding requests in the httpd receive buffer after sending the response, and if so, process them. Test that pipelined requests are handled by sending multiple minimal HTTP/1.1 using netcat (nc) and checking that we get back the same number of responses. --- bin/tests/system/conf.sh.in | 3 ++ bin/tests/system/statistics/tests.sh | 8 ++++-- bin/tests/system/statschannel/clean.sh | 1 + bin/tests/system/statschannel/tests.sh | 21 ++++++++++++++ configure.ac | 7 +++++ lib/isc/httpd.c | 40 +++++++++++++++++++++----- 6 files changed, 71 insertions(+), 9 deletions(-) diff --git a/bin/tests/system/conf.sh.in b/bin/tests/system/conf.sh.in index 06aaad39c2..ff6c6e4b67 100644 --- a/bin/tests/system/conf.sh.in +++ b/bin/tests/system/conf.sh.in @@ -112,6 +112,9 @@ SHELL=@SHELL@ # CURL will be empty if no program was found by configure CURL=@CURL@ +# NC will be empty if no program was found by configure +NC=@NC@ + # XMLLINT will be empty if no program was found by configure XMLLINT=@XMLLINT@ diff --git a/bin/tests/system/statistics/tests.sh b/bin/tests/system/statistics/tests.sh index 4cb8008180..7e0190c604 100644 --- a/bin/tests/system/statistics/tests.sh +++ b/bin/tests/system/statistics/tests.sh @@ -175,8 +175,12 @@ echo_i "checking bind9.xsl vs xml ($n)" if $FEATURETEST --have-libxml2 && [ -x "${CURL}" ] && [ -x "${XSLTPROC}" ] ; then $DIGCMD +notcp +recurse @10.53.0.3 soa . > dig.out.test$n.1 2>&1 $DIGCMD +notcp +recurse @10.53.0.3 soa example > dig.out.test$n.2 2>&1 - ${CURL} http://10.53.0.3:${EXTRAPORT1}/xml/v3 > curl.out.${n}.xml 2>/dev/null || ret=1 - ${CURL} http://10.53.0.3:${EXTRAPORT1}/bind9.xsl > curl.out.${n}.xsl 2>/dev/null || ret=1 + # check multiple requests over the same socket + time1=$($PERL -e 'print time(), "\n";') + ${CURL} --http1.1 -o curl.out.${n}.xml http://10.53.0.3:${EXTRAPORT1}/xml/v3 \ + -o curl.out.${n}.xsl http://10.53.0.3:${EXTRAPORT1}/bind9.xsl 2>/dev/null || ret=1 + time2=$($PERL -e 'print time(), "\n";') + test $((time2 - time1)) -lt 5 || ret=1 ${DIFF} ${TOP_SRCDIR}/bin/named/bind9.xsl curl.out.${n}.xsl || ret=1 ${XSLTPROC} curl.out.${n}.xsl - < curl.out.${n}.xml > xsltproc.out.${n} 2>/dev/null || ret=1 cp curl.out.${n}.xml stats.xml.out || ret=1 diff --git a/bin/tests/system/statschannel/clean.sh b/bin/tests/system/statschannel/clean.sh index 2e13b1a9a5..03cb1bb87d 100644 --- a/bin/tests/system/statschannel/clean.sh +++ b/bin/tests/system/statschannel/clean.sh @@ -28,3 +28,4 @@ rm -f xml.*mem json.*mem rm -f xml.*stats json.*stats rm -f zones zones.out.* zones.json.* zones.xml.* zones.expect.* rm -rf ./__pycache__ +rm -f nc.out* diff --git a/bin/tests/system/statschannel/tests.sh b/bin/tests/system/statschannel/tests.sh index bbbc8a0c97..ce145b7c3e 100644 --- a/bin/tests/system/statschannel/tests.sh +++ b/bin/tests/system/statschannel/tests.sh @@ -374,5 +374,26 @@ if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` n=`expr $n + 1` +if [ -x "${NC}" ] ; then + echo_i "Check HTTP/1.1 pipelined requests are handled ($n)" + ret=0 + ${NC} 10.53.0.3 ${EXTRAPORT1} << EOF > nc.out$n || ret=1 +GET /xml/v3/status HTTP/1.1 +Host: 10.53.0.3:${EXTRAPORT1} + +GET /xml/v3/status HTTP/1.1 +Host: 10.53.0.3:${EXTRAPORT1} +Connection: close + +EOF + lines=$(grep "^HTTP/1.1" nc.out$n | wc -l) + test $lines = 2 || ret=1 + if [ $ret != 0 ]; then echo_i "failed"; fi + status=`expr $status + $ret` + n=`expr $n + 1` +else + echo_i "skipping test as nc not found" +fi + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 diff --git a/configure.ac b/configure.ac index fa1dbae162..17ca8eb60c 100644 --- a/configure.ac +++ b/configure.ac @@ -1256,6 +1256,13 @@ AC_CONFIG_FILES([doc/doxygen/doxygen-input-filter], AC_PATH_PROG(CURL, curl, curl) AC_SUBST(CURL) +# +# Look for nc +# + +AC_PATH_PROGS(NC, nc, nc) +AC_SUBST(NC) + # # IDN support using libidn2 # diff --git a/lib/isc/httpd.c b/lib/isc/httpd.c index 9596b8e0af..0787da8c18 100644 --- a/lib/isc/httpd.c +++ b/lib/isc/httpd.c @@ -403,11 +403,14 @@ process_request(isc_httpd_t *httpd, isc_region_t *region, size_t *buflen) { len = limit; } - memmove(httpd->recvbuf + httpd->recvlen, region->base, len); - httpd->recvlen += len; - httpd->recvbuf[httpd->recvlen] = 0; - *buflen = httpd->recvlen; + if (len > 0U) { + memmove(httpd->recvbuf + httpd->recvlen, region->base, len); + httpd->recvlen += len; + httpd->recvbuf[httpd->recvlen] = 0; + isc_region_consume(region, len); + } httpd->headers = NULL; + *buflen = httpd->recvlen; /* * If we don't find a blank line in our buffer, return that we need @@ -858,10 +861,22 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, goto cleanup_readhandle; } - result = process_request(httpd, region, &buflen); + result = process_request( + httpd, region == NULL ? &(isc_region_t){ NULL, 0 } : region, + &buflen); if (result == ISC_R_NOTFOUND) { if (buflen < HTTP_RECVLEN - 1) { - /* don't unref, keep reading */ + if (region != NULL) { + /* don't unref, keep reading */ + return; + } + /* + * We have been called from httpd_senddone + * and we need to resume reading. Detach + * readhandle before resuming. + */ + isc_nmhandle_detach(&httpd->readhandle); + isc_nm_resumeread(handle); return; } goto cleanup_readhandle; @@ -950,6 +965,7 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, * the response headers and store the result in httpd->sendbuffer. */ isc_buffer_dup(mgr->mctx, &httpd->sendbuffer, &httpd->headerbuffer); + isc_buffer_clear(&httpd->headerbuffer); isc_buffer_setautorealloc(httpd->sendbuffer, true); databuffer = (is_compressed ? &httpd->compbuffer : &httpd->bodybuffer); isc_buffer_usedregion(databuffer, &r); @@ -965,6 +981,7 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, } httpd->recvlen -= httpd->consume; httpd->consume = 0; + httpd->recvbuf[httpd->recvlen] = 0; } /* @@ -1150,7 +1167,16 @@ httpd_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { } httpd->state = RECV; - isc_nm_resumeread(handle); + if (httpd->recvlen != 0) { + /* + * Outstanding requests still exist, start processing + * them. + */ + isc_nmhandle_attach(handle, &httpd->readhandle); + httpd_request(handle, ISC_R_SUCCESS, NULL, httpd->mgr); + } else { + isc_nm_resumeread(handle); + } } isc_result_t From 0b83f1495df569d38a95652b38e18588ec2178bc Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 4 Nov 2021 14:36:33 +1100 Subject: [PATCH 4/5] Handle truncating the request stream in isc_httpd If we have had to truncate the request stream, don't resume reading from it. --- bin/tests/system/statschannel/clean.sh | 1 + bin/tests/system/statschannel/send64k.pl | 44 ++++++++++++++++++++++++ bin/tests/system/statschannel/tests.sh | 27 +++++++++++++++ lib/isc/httpd.c | 15 ++++++-- util/copyrights | 1 + 5 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 bin/tests/system/statschannel/send64k.pl diff --git a/bin/tests/system/statschannel/clean.sh b/bin/tests/system/statschannel/clean.sh index 03cb1bb87d..09218d5b5b 100644 --- a/bin/tests/system/statschannel/clean.sh +++ b/bin/tests/system/statschannel/clean.sh @@ -29,3 +29,4 @@ rm -f xml.*stats json.*stats rm -f zones zones.out.* zones.json.* zones.xml.* zones.expect.* rm -rf ./__pycache__ rm -f nc.out* +rm -f send.in* send.out* diff --git a/bin/tests/system/statschannel/send64k.pl b/bin/tests/system/statschannel/send64k.pl new file mode 100644 index 0000000000..b382dd59da --- /dev/null +++ b/bin/tests/system/statschannel/send64k.pl @@ -0,0 +1,44 @@ +#!/usr/bin/perl +# +# Copyright (C) Internet Systems Consortium, Inc. ("ISC") +# +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, you can obtain one at https://mozilla.org/MPL/2.0/. +# +# See the COPYRIGHT file distributed with this work for additional +# information regarding copyright ownership. + +# +# Send a file to a given address and port using TCP. Used for +# configuring the test server in ans.pl. +# + +use IO::File; +use IO::Socket; + +@ARGV == 2 or die "usage: send.pl host port\n"; + +my $host = shift @ARGV; +my $port = shift @ARGV; + +my $sock = IO::Socket::INET->new(PeerAddr => $host, PeerPort => $port, + Proto => "tcp",) or die "$!"; +#send the file +while ($n = read(STDIN, $buf, 64000)) { + $sock->syswrite($buf, $n); +} + +#get the response with with a 15 second timeout +my $rin; +my $rout; +my $n; +do { + $rin = ''; + vec($rin, fileno($sock), 1) = 1; + $n = select($rout = $rin, undef, undef, 15); + $n = $sock->sysread($buf, 64000) if ($n > 0); + print STDOUT $buf if ($n > 0); +} while ($n > 0); + +$sock->close; diff --git a/bin/tests/system/statschannel/tests.sh b/bin/tests/system/statschannel/tests.sh index ce145b7c3e..2ce484b5db 100644 --- a/bin/tests/system/statschannel/tests.sh +++ b/bin/tests/system/statschannel/tests.sh @@ -395,5 +395,32 @@ else echo_i "skipping test as nc not found" fi +echo_i "Check HTTP/1.1 pipelined with truncated stream ($n)" +ret=0 +i=0 +# build input stream. +cp /dev/null send.in$n +while test $i -lt 500 +do +cat >> send.in$n << EOF +GET /xml/v3/status HTTP/1.1 +Host: 10.53.0.3 + +EOF +i=$((i+1)) +done + +# send the requests then wait for named to close the socket. +time1=$($PERL -e 'print time(), "\n";') +${PERL} send64k.pl 10.53.0.3 ${EXTRAPORT1} < send.in$n > send.out$n +time2=$($PERL -e 'print time(), "\n";') +test $((time2 - time1)) -lt 5 || ret=1 +# we expect 91 of the 500 requests to be processed. +lines=$(grep "^HTTP/1.1" send.out$n | wc -l) +test $lines = 91 || ret=1 +if [ $ret != 0 ]; then echo_i "failed"; fi +status=`expr $status + $ret` +n=`expr $n + 1` + echo_i "exit status: $status" [ $status -eq 0 ] || exit 1 diff --git a/lib/isc/httpd.c b/lib/isc/httpd.c index 0787da8c18..1d79668b91 100644 --- a/lib/isc/httpd.c +++ b/lib/isc/httpd.c @@ -89,6 +89,7 @@ struct isc_httpd { uint32_t recvlen; /*%< length recv'd */ uint32_t consume; /*%< length of last command */ char *headers; /*%< set in process_request() */ + bool truncated; method_t method; char *url; char *querystring; @@ -398,17 +399,25 @@ process_request(isc_httpd_t *httpd, isc_region_t *region, size_t *buflen) { size_t limit = sizeof(httpd->recvbuf) - httpd->recvlen - 1; size_t len = region->length; int delim; + bool truncated = false; if (len > limit) { len = limit; + truncated = true; } if (len > 0U) { + if (httpd->truncated) { + return (ISC_R_NOSPACE); + } memmove(httpd->recvbuf + httpd->recvlen, region->base, len); httpd->recvlen += len; httpd->recvbuf[httpd->recvlen] = 0; isc_region_consume(region, len); } + if (truncated) { + httpd->truncated = true; + } httpd->headers = NULL; *buflen = httpd->recvlen; @@ -422,7 +431,8 @@ process_request(isc_httpd_t *httpd, isc_region_t *region, size_t *buflen) { s = strstr(httpd->recvbuf, "\n\n"); delim = 1; if (s == NULL) { - return (ISC_R_NOTFOUND); + return (httpd->truncated ? ISC_R_NOSPACE + : ISC_R_NOTFOUND); } httpd->consume = s + 2 - httpd->recvbuf; } else { @@ -609,6 +619,7 @@ httpd_reset(void *arg) { httpd->recvbuf[0] = 0; httpd->recvlen = 0; httpd->consume = 0; + httpd->truncated = false; httpd->headers = NULL; httpd->method = METHOD_UNKNOWN; httpd->url = NULL; @@ -1174,7 +1185,7 @@ httpd_senddone(isc_nmhandle_t *handle, isc_result_t result, void *arg) { */ isc_nmhandle_attach(handle, &httpd->readhandle); httpd_request(handle, ISC_R_SUCCESS, NULL, httpd->mgr); - } else { + } else if (!httpd->truncated) { isc_nm_resumeread(handle); } } diff --git a/util/copyrights b/util/copyrights index ede8082947..36a154d760 100644 --- a/util/copyrights +++ b/util/copyrights @@ -765,6 +765,7 @@ ./bin/tests/system/statschannel/helper.py PYTHON 2020,2021 ./bin/tests/system/statschannel/mem-xml.pl PERL 2017,2018,2019,2020,2021 ./bin/tests/system/statschannel/ns2/sign.sh SH 2019,2020,2021 +./bin/tests/system/statschannel/send64k.pl PERL 2021 ./bin/tests/system/statschannel/server-json.pl PERL 2015,2016,2017,2018,2019,2020,2021 ./bin/tests/system/statschannel/server-xml.pl PERL 2015,2016,2017,2018,2019,2020,2021 ./bin/tests/system/statschannel/setup.sh SH 2018,2019,2020,2021 From 5f05cf97c7f86fff2ad77c8d13c1f21cf10e7c60 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Fri, 22 Oct 2021 22:22:38 -0700 Subject: [PATCH 5/5] CHANGES for [GL #2973] --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index e2e7dff829..3432aadf15 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5755. [bug] The statistics channel wasn't correctly handling + multiple HTTP requests, or pipelined or truncated + requests. [GL #2973] + 5754. [bug] "tls" statements may omit "key-file" and "cert-file", but if either one is specified, then both must be. [GL #2986]