From b7e28a6a1c5e2664cdaeff7e7f5ac39aebfc12d0 Mon Sep 17 00:00:00 2001 From: Noah Sanci Date: Mon, 26 Jul 2021 13:29:11 -0400 Subject: [PATCH] debuginfod: PR27982 - added DEBUGINFOD_MAXSIZE and DEBUGINFOD_MAXTIME DEBUGINFOD_TIMEOUT is a good way to catch servers that are too slow to *start* transmitting a file. But we have no way of limiting total download time or space. A user might prefer to have his debugger fetch only quick & small files, and make do without the bigger ones. Some transitive dependencies of e.g. gnome programs are huge: 3GB of LLVM debuginfo, 1GB of webkitgtk, etc. etc. DEBUGINFOD_MAXSIZE and DEBUGINFOD_MAXTIME were added to dictate the max download size and time of a debuginfod client. DEBUGINFOD_MAXSIZE is handled server-side and is sent using the http header: X-DEBUGINFOD-MAXSIZE. The client side then checks to ensure this maxsize has been respected. https://sourceware.org/bugzilla/show_bug.cgi?id=27982 Signed-off-by: Noah Sanci --- debuginfod/ChangeLog | 12 +++++ debuginfod/debuginfod-client.c | 99 ++++++++++++++++++++++++++++++++-- debuginfod/debuginfod.cxx | 13 +++++ debuginfod/debuginfod.h.in | 2 + doc/ChangeLog | 6 +++ doc/debuginfod-find.1 | 15 ++++++ tests/ChangeLog | 7 +++ tests/run-debuginfod-find.sh | 26 +++++++++ 8 files changed, 177 insertions(+), 3 deletions(-) diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 81eb56f9..9e82d78d 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,15 @@ +2021-07-26 Noah Sanci + + PR27982 + * debuginfod-client.c (globals): added default_maxsize and + default_maxtime. + (debuginfod_query_server): Added DEBUGINFOD_MAXSIZE and + DEBUGINFOD_MAXTIME envvar processing. + * debuginfod.cxx (handler_cb): If the requested file exceeds + maxsize return code 406. + * debuginfod.h.in: Added DEBUGINFOD_MAXSIZE_ENV_VAR and + DEBUGINFOD_MAXTIME_ENV_VAR. + 2021-07-16 Noah Sanci PR28034 diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index a70f6d79..7d4b220f 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -555,6 +555,39 @@ debuginfod_query_server (debuginfod_client *c, free (c->url); c->url = NULL; + /* PR 27982: Add max size if DEBUGINFOD_MAXSIZE is set. */ + long maxsize = 0; + const char *maxsize_envvar; + maxsize_envvar = getenv(DEBUGINFOD_MAXSIZE_ENV_VAR); + if (maxsize_envvar != NULL) + maxsize = atol (maxsize_envvar); + + /* PR 27982: Add max time if DEBUGINFOD_MAXTIME is set. */ + long maxtime = 0; + const char *maxtime_envvar; + maxtime_envvar = getenv(DEBUGINFOD_MAXTIME_ENV_VAR); + if (maxtime_envvar != NULL) + maxtime = atol (maxtime_envvar); + if (maxtime && vfd >= 0) + dprintf(vfd, "using max time %lds\n", maxtime); + + /* Maxsize is valid*/ + if (maxsize > 0) + { + if (vfd) + dprintf (vfd, "using max size %ldB\n", maxsize); + char *size_header = NULL; + rc = asprintf (&size_header, "X-DEBUGINFOD-MAXSIZE: %ld", maxsize); + if (rc < 0) + { + rc = -ENOMEM; + goto out; + } + rc = debuginfod_add_http_header(c, size_header); + free(size_header); + if (rc < 0) + goto out; + } add_default_headers(c); /* Copy lowercase hex representation of build_id into buf. */ @@ -833,7 +866,7 @@ debuginfod_query_server (debuginfod_client *c, if (data == NULL) { rc = -ENOMEM; - goto out0; + goto out1; } /* thereafter, goto out1 on error. */ @@ -864,7 +897,7 @@ debuginfod_query_server (debuginfod_client *c, if (data[i].handle == NULL) { rc = -ENETUNREACH; - goto out2; + goto out1; } data[i].client = c; @@ -876,7 +909,7 @@ debuginfod_query_server (debuginfod_client *c, if (!escaped_string) { rc = -ENOMEM; - goto out1; + goto out2; } snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url, build_id_bytes, type, escaped_string); @@ -927,8 +960,31 @@ debuginfod_query_server (debuginfod_client *c, long loops = 0; int committed_to = -1; bool verbose_reported = false; + struct timespec start_time, cur_time; + if ( maxtime > 0 && clock_gettime(CLOCK_MONOTONIC_RAW, &start_time) == -1) + { + rc = errno; + goto out2; + } + long delta = 0; do { + /* Check to see how long querying is taking. */ + if (maxtime > 0) + { + if (clock_gettime(CLOCK_MONOTONIC_RAW, &cur_time) == -1) + { + rc = errno; + goto out2; + } + delta = cur_time.tv_sec - start_time.tv_sec; + if ( delta > maxtime) + { + dprintf(vfd, "Timeout with max time=%lds and transfer time=%lds\n", maxtime, delta ); + rc = -ETIME; + goto out2; + } + } /* Wait 1 second, the minimum DEBUGINFOD_TIMEOUT. */ curl_multi_wait(curlm, NULL, 0, 1000, NULL); @@ -1010,6 +1066,29 @@ debuginfod_query_server (debuginfod_client *c, if ((*c->progressfn) (c, pa, pb)) break; } + /* Check to see if we are downloading something which exceeds maxsize, if set.*/ + if (maxsize > 0 && target_handle) + { + long dl_size = 0; +#ifdef CURLINFO_SIZE_DOWNLOAD_T + curl_off_t download_size_t; + if (curl_easy_getinfo(target_handle, CURLINFO_CONTENT_LENGTH_DOWNLOAD_T, + &download_size_t) == CURLE_OK) + dl_size = download_size_t >= (double)(LONG_MAX+1UL) ? LONG_MAX : (long)download_size_t; +#else + double download_size; + if (curl_easy_getinfo(target_handle, CURLINFO_CONTENT_LENGTH_DOWNLOAD, + &download_size) == CURLE_OK) + dl_size = download_size >= (double)(LONG_MAX+1UL) ? LONG_MAX : (long)download_size; +#endif + if (dl_size > maxsize) + { + if (vfd >=0) + dprintf(vfd, "Content-Length too large.\n"); + rc = -EFBIG; + goto out2; + } + } } while (still_running); /* Check whether a query was successful. If so, assign its handle @@ -1043,6 +1122,8 @@ debuginfod_query_server (debuginfod_client *c, if (msg->data.result != CURLE_OK) { + long resp_code; + CURLcode ok0; /* Unsuccessful query, determine error code. */ switch (msg->data.result) { @@ -1057,6 +1138,16 @@ debuginfod_query_server (debuginfod_client *c, case CURLE_SEND_ERROR: rc = -ECONNRESET; break; case CURLE_RECV_ERROR: rc = -ECONNRESET; break; case CURLE_OPERATION_TIMEDOUT: rc = -ETIME; break; + case CURLE_HTTP_RETURNED_ERROR: + ok0 = curl_easy_getinfo (msg->easy_handle, + CURLINFO_RESPONSE_CODE, + &resp_code); + /* 406 signals that the requested file was too large */ + if ( ok0 == CURLE_OK && resp_code == 406) + rc = -EFBIG; + else + rc = -ENOENT; + break; default: rc = -ENOENT; break; } } @@ -1129,6 +1220,8 @@ debuginfod_query_server (debuginfod_client *c, if (efd >= 0) close(efd); } + else if (rc == -EFBIG) + goto out2; /* If the verified_handle is NULL and rc != -ENOENT, the query fails with * an error code other than 404, then do several retry within the retry_limit. diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx index 503edba7..4ddd9255 100644 --- a/debuginfod/debuginfod.cxx +++ b/debuginfod/debuginfod.cxx @@ -2102,6 +2102,13 @@ handler_cb (void * /*cls*/, } *ptr = NULL; /* reset when done */ + const char *maxsize_string = MHD_lookup_connection_value(connection, MHD_HEADER_KIND, "X-DEBUGINFOD-MAXSIZE"); + long maxsize = 0; + if (maxsize_string != NULL && maxsize_string[0] != '\0') + maxsize = atol(maxsize_string); + else + maxsize = 0; + #if MHD_VERSION >= 0x00097002 enum MHD_Result rc; #else @@ -2190,6 +2197,12 @@ handler_cb (void * /*cls*/, if (r == 0) throw reportable_exception("internal error, missing response"); + if (maxsize > 0 && http_size > maxsize) + { + MHD_destroy_response(r); + throw reportable_exception(406, "File too large, max size=" + std::to_string(maxsize)); + } + rc = MHD_queue_response (connection, MHD_HTTP_OK, r); http_code = MHD_HTTP_OK; MHD_destroy_response (r); diff --git a/debuginfod/debuginfod.h.in b/debuginfod/debuginfod.h.in index 282e523d..c358df4d 100644 --- a/debuginfod/debuginfod.h.in +++ b/debuginfod/debuginfod.h.in @@ -36,6 +36,8 @@ #define DEBUGINFOD_PROGRESS_ENV_VAR "DEBUGINFOD_PROGRESS" #define DEBUGINFOD_VERBOSE_ENV_VAR "DEBUGINFOD_VERBOSE" #define DEBUGINFOD_RETRY_LIMIT_ENV_VAR "DEBUGINFOD_RETRY_LIMIT" +#define DEBUGINFOD_MAXSIZE_ENV_VAR "DEBUGINFOD_MAXSIZE" +#define DEBUGINFOD_MAXTIME_ENV_VAR "DEBUGINFOD_MAXTIME" /* The libdebuginfod soname. */ #define DEBUGINFOD_SONAME "@LIBDEBUGINFOD_SONAME@" diff --git a/doc/ChangeLog b/doc/ChangeLog index 05fcd23d..1822fc6b 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -1,3 +1,9 @@ +2021-07-26 Noah Sanci + + PR27982 + * debuginfod-find.1: Document DEBUGINFOD_MAXTIME + and DEBUGINFOD_MAXSIZE. + 2021-04-23 Frank Ch. Eigler PR27701 diff --git a/doc/debuginfod-find.1 b/doc/debuginfod-find.1 index 12d4ec2d..482a8ae7 100644 --- a/doc/debuginfod-find.1 +++ b/doc/debuginfod-find.1 @@ -147,6 +147,21 @@ is reexecuted. Cache management parameters may be set by files under this directory: see the \fBdebuginfod_find_debuginfo(3)\fP man page for details. The default is $HOME/.debuginfod_client_cache. +.TP 21 +.B DEBUGINFOD_MAXTIME +This environment variable dictates how long the client will wait to +download a file found on a server in seconds. It is best used to ensure +that a file is downloaded quickly or be rejected. The default is +0 (infinite time). + +.TP 21 +.B DEBUGINFOD_MAXSIZE +This environment variable dictates the maximum size of a file to +download in bytes. This is best used if the user would like to ensure +only small files are downloaded. A value of 0 causes no consideration +for size, and the client may attempt to download a file of any size. +The default is 0 (infinite size). + .SH "FILES" .LP .PD .1v diff --git a/tests/ChangeLog b/tests/ChangeLog index dba750e4..34666609 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,10 @@ +2021-07-26 Noah Sanci + + PR27982 + * run-debuginfod-find.sh: Added a test to ensure that + DEBUGINFOD_MAXSIZE and DEBUGINFOD_MAXTIME work properly + by searching server and client logs for prompts. + 2021-07-16 Noah Sanci PR28034 diff --git a/tests/run-debuginfod-find.sh b/tests/run-debuginfod-find.sh index 44e16242..991d1dc5 100755 --- a/tests/run-debuginfod-find.sh +++ b/tests/run-debuginfod-find.sh @@ -177,7 +177,33 @@ wait_ready $PORT1 'thread_busy{role="scan"}' 0 testrun ${abs_builddir}/debuginfod_build_id_find -e F/p+r%o\$g 1 ######################################################################## +## PR27892 +# Ensure DEBUGINFOD_MAXSIZE is functional and sends back the correct http +# code +env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_RETRY_LIMIT=1 DEBUGINFOD_URLS="http://127.0.0.1:$PORT1/" DEBUGINFOD_MAXSIZE=1 \ + ${abs_top_builddir}/debuginfod/debuginfod-find -v debuginfo F/p+r%o\$g.debug 2> find-vlog$PORT1 || true +tempfiles find-vlog$PORT1 +# wait for the server to fail the same number of times the query is retried. +wait_ready $PORT1 'http_responses_after_you_milliseconds_count{code="406"}' 1 +# ensure all reporting is functional +grep 'serving file '$(realpath ${PWD})'/F/p+r%o\$g.debug' vlog$PORT1 +grep 'File too large' vlog$PORT1 +grep 'using max size 1B' find-vlog$PORT1 +if [ -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID} ]; then + echo "File cached after maxsize check" + err +fi +# Ensure DEBUGINFOD_MAXTIME is functional +env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS="http://127.0.0.1:8002/" DEBUGINFOD_MAXTIME=1 \ + ${abs_top_builddir}/debuginfod/debuginfod-find -v debuginfo F/p+r%o\$g.debug 2> find-vlog$PORT1 || true +grep 'using max time' find-vlog$PORT1 +# Ensure p+r%o\$g.debug is NOT cached +if [ -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID} ]; then + echo "File cached after maxtime check" + err +fi +######################################################################## # PR25628 rm -rf $DEBUGINFOD_CACHE_PATH # clean it from previous tests -- 2.31.1