From 979f19eb4fd7a35ace4ddafed103922559b93120 Mon Sep 17 00:00:00 2001 From: Noah Sanci Date: Wed, 28 Jul 2021 14:46:05 -0400 Subject: [PATCH 2/2] debuginfod: PR27277 - Describe retrieved files when verbose Allow users, with enough verbosity, to print the HTTP response headers upon retrieving a file. These files may include several custome http response headers such as X-DEBUGINFOD-FILE, X-DEBUGINFOD-SIZE, and X-DEBUGINFOD-ARCHIVE. These headers are added from the daemon, in debuginfod.cxx. run-debuginfod-fd-prefetch-caches.sh was updated so that it doesn't trip and fail as previously greping for a value that should yield zero caused an error. Previously, target_handle was used to gather CURLE_OK reponses. Under some conditions, target_handle was NULL when we wanted it to point to the handle. This could cause some failuers. instead msg->easy_handle is used, which ensures the correct handle is used. E.g output: HTTP/1.1 200 OK Connection: Keep-Alive Content-Length: 4095072 Cache-Control: public Last-Modified: Thu, 09 Sep 2021 19:06:40 GMT X-FILE: debuginfod X-FILE-SIZE: 4095072 Content-Type: application/octet-stream Date: Fri, 10 Sep 2021 16:38:06 GMT https://sourceware.org/bugzilla/show_bug.cgi?id=27277 Signed-off-by: Noah Sanci --- debuginfod/ChangeLog | 16 +++ debuginfod/debuginfod-client.c | 58 +++++++++- debuginfod/debuginfod.cxx | 11 ++ doc/ChangeLog | 8 ++ doc/debuginfod-find.1 | 3 +- doc/debuginfod.8 | 9 ++ tests/ChangeLog | 18 +++- tests/Makefile.am | 3 +- tests/run-debuginfod-fd-prefetch-caches.sh | 6 +- tests/run-debuginfod-response-headers.sh | 118 +++++++++++++++++++++ tests/test-subr.sh | 10 +- 11 files changed, 241 insertions(+), 19 deletions(-) create mode 100755 tests/run-debuginfod-response-headers.sh diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 7e221f54..1ec83de9 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -26,6 +26,22 @@ * debuginfod.cxx (handler_cb): Fix after_you unique_set key to the entire incoming URL. +2021-08-02 Noah Sanci + + PR27277 + * debuginfod-client.c (struct debuginfod_client): New field + winning_headers. + (struct handle_data): New field response_data. + (header_callback): Store received headers in response_data. + (debuginfod_query_server): Activate CURLOPT_HEADERFUNCTION. + Save winning response_data. + (debuginfod_end): free client winning headers. + * debuginfod.cxx (handle_buildid_f_match): remove X-DEBUGINFOD-FILE + path. Add X-DEBUGINFOD-FILE and X-DEBUGINFOD-SIZE headers. + (handle_buildid_r_match): remove X-DEBUGINFOD-FILE path. Add + X-DEBUGINFOD-FILE, X-DEBUGINFOD-SIZE + headers, and X-ARCHIVE headers. + 2021-07-26 Noah Sanci PR27982 diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c index d41723ce..9c16e7a3 100644 --- a/debuginfod/debuginfod-client.c +++ b/debuginfod/debuginfod-client.c @@ -127,6 +127,7 @@ struct debuginfod_client timeout or other info gotten from environment variables, the handle data, etc. So those don't have to be reparsed and recreated on each request. */ + char * winning_headers; }; /* The cache_clean_interval_s file within the debuginfod cache specifies @@ -183,6 +184,8 @@ struct handle_data to the cache. Used to ensure that a file is not downloaded from multiple servers unnecessarily. */ CURL **target_handle; + /* Response http headers for this client handle, sent from the server */ + char *response_data; }; static size_t @@ -499,6 +502,33 @@ default_progressfn (debuginfod_client *c, long a, long b) } +static size_t +header_callback (char * buffer, size_t size, size_t numitems, void * userdata) +{ + if (size != 1) + return 0; + /* Temporary buffer for realloc */ + char *temp = NULL; + size_t userlen = 0; + if (*(char**)userdata == NULL) + { + temp = malloc(numitems+1); + if (temp == NULL) + return 0; + memset(temp, '\0', numitems+1); + } + else + { + userlen = strlen(*(char**)userdata); + temp = realloc(*(char**)userdata, userlen + numitems + 1); + if (temp == NULL) + return 0; + } + strncat(temp, buffer, numitems); + *(char**)userdata = temp; + return numitems; +} + /* Query each of the server URLs found in $DEBUGINFOD_URLS for the file with the specified build-id, type (debuginfo, executable or source) and filename. filename may be NULL. If found, return a file @@ -936,10 +966,13 @@ debuginfod_query_server (debuginfod_client *c, curl_easy_setopt (data[i].handle, CURLOPT_LOW_SPEED_LIMIT, 100 * 1024L); } + data[i].response_data = NULL; curl_easy_setopt(data[i].handle, CURLOPT_FILETIME, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_FOLLOWLOCATION, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_FAILONERROR, (long) 1); curl_easy_setopt(data[i].handle, CURLOPT_NOSIGNAL, (long) 1); + curl_easy_setopt(data[i].handle, CURLOPT_HEADERFUNCTION, header_callback); + curl_easy_setopt(data[i].handle, CURLOPT_HEADERDATA, (void *) &(data[i].response_data)); #if LIBCURL_VERSION_NUM >= 0x072a00 /* 7.42.0 */ curl_easy_setopt(data[i].handle, CURLOPT_PATH_AS_IS, (long) 1); #else @@ -961,6 +994,7 @@ debuginfod_query_server (debuginfod_client *c, int committed_to = -1; bool verbose_reported = false; struct timespec start_time, cur_time; + c->winning_headers = NULL; if ( maxtime > 0 && clock_gettime(CLOCK_MONOTONIC_RAW, &start_time) == -1) { rc = errno; @@ -995,8 +1029,18 @@ debuginfod_query_server (debuginfod_client *c, if (data[i].handle != target_handle) curl_multi_remove_handle(curlm, data[i].handle); else - committed_to = i; - } + { + committed_to = i; + if (c->winning_headers == NULL) + { + c->winning_headers = data[committed_to].response_data; + if (vfd >= 0 && c->winning_headers != NULL) + dprintf(vfd, "\n%s", c->winning_headers); + data[committed_to].response_data = NULL; + } + + } + } if (vfd >= 0 && !verbose_reported && committed_to >= 0) { @@ -1161,10 +1205,10 @@ debuginfod_query_server (debuginfod_client *c, { char *effective_url = NULL; long resp_code = 500; - CURLcode ok1 = curl_easy_getinfo (target_handle, + CURLcode ok1 = curl_easy_getinfo (msg->easy_handle, CURLINFO_EFFECTIVE_URL, &effective_url); - CURLcode ok2 = curl_easy_getinfo (target_handle, + CURLcode ok2 = curl_easy_getinfo (msg->easy_handle, CURLINFO_RESPONSE_CODE, &resp_code); if(ok1 == CURLE_OK && ok2 == CURLE_OK && effective_url) @@ -1238,7 +1282,10 @@ debuginfod_query_server (debuginfod_client *c, { curl_multi_remove_handle(curlm, data[i].handle); /* ok to repeat */ curl_easy_cleanup (data[i].handle); + free(data[i].response_data); } + free(c->winning_headers); + c->winning_headers = NULL; goto query_in_parallel; } else @@ -1281,6 +1328,7 @@ debuginfod_query_server (debuginfod_client *c, { curl_multi_remove_handle(curlm, data[i].handle); /* ok to repeat */ curl_easy_cleanup (data[i].handle); + free (data[i].response_data); } for (int i = 0; i < num_urls; ++i) @@ -1304,6 +1352,7 @@ debuginfod_query_server (debuginfod_client *c, { curl_multi_remove_handle(curlm, data[i].handle); /* ok to repeat */ curl_easy_cleanup (data[i].handle); + free (data[i].response_data); } unlink (target_cache_tmppath); @@ -1415,6 +1464,7 @@ debuginfod_end (debuginfod_client *client) curl_multi_cleanup (client->server_mhandle); curl_slist_free_all (client->headers); + free (client->winning_headers); free (client->url); free (client); } diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx index 3269f657..4f7a09ad 100644 --- a/debuginfod/debuginfod.cxx +++ b/debuginfod/debuginfod.cxx @@ -1075,6 +1075,9 @@ handle_buildid_f_match (bool internal_req_t, else { MHD_add_response_header (r, "Content-Type", "application/octet-stream"); + std::string file = b_source0.substr(b_source0.find_last_of("/")+1, b_source0.length()); + MHD_add_response_header (r, "X-DEBUGINFOD-SIZE", to_string(s.st_size).c_str() ); + MHD_add_response_header (r, "X-DEBUGINFOD-FILE", file.c_str() ); add_mhd_last_modified (r, s.st_mtime); if (verbose > 1) obatched(clog) << "serving file " << b_source0 << endl; @@ -1544,6 +1547,9 @@ handle_buildid_r_match (bool internal_req_p, inc_metric ("http_responses_total","result","archive fdcache"); MHD_add_response_header (r, "Content-Type", "application/octet-stream"); + MHD_add_response_header (r, "X-DEBUGINFOD-SIZE", to_string(fs.st_size).c_str()); + MHD_add_response_header (r, "X-DEBUGINFOD-ARCHIVE", b_source0.c_str()); + MHD_add_response_header (r, "X-DEBUGINFOD-FILE", b_source1.c_str()); add_mhd_last_modified (r, fs.st_mtime); if (verbose > 1) obatched(clog) << "serving fdcache archive " << b_source0 << " file " << b_source1 << endl; @@ -1685,6 +1691,11 @@ handle_buildid_r_match (bool internal_req_p, else { MHD_add_response_header (r, "Content-Type", "application/octet-stream"); + std::string file = b_source1.substr(b_source1.find_last_of("/")+1, b_source1.length()); + MHD_add_response_header (r, "X-DEBUGINFOD-SIZE", to_string(fs.st_size).c_str()); + MHD_add_response_header (r, "X-DEBUGINFOD-ARCHIVE", b_source0.c_str()); + MHD_add_response_header (r, "X-DEBUGINFOD-FILE", file.c_str()); + add_mhd_last_modified (r, archive_entry_mtime(e)); if (verbose > 1) obatched(clog) << "serving archive " << b_source0 << " file " << b_source1 << endl; diff --git a/doc/ChangeLog b/doc/ChangeLog index ada48383..db3a3584 100644 --- a/doc/ChangeLog +++ b/doc/ChangeLog @@ -18,6 +18,14 @@ * Makefile.am: Updated to include debuginfod-client-config.7 * man3, man7: Symlinks for source tree man page testing. +2021-08-04 Noah Sanci + + PR27277 + * debuginfod-find.1: Increasing verbosity describes the downloaded + file. + * debuginfod.8: Describe X-DEBUGINFOD-FILE, X-DEBUGINFOD-SIZE, and + X-DEBUGINFOD-ARCHIVE. + 2021-07-26 Noah Sanci PR27982 diff --git a/doc/debuginfod-find.1 b/doc/debuginfod-find.1 index a61673f5..957ec7e7 100644 --- a/doc/debuginfod-find.1 +++ b/doc/debuginfod-find.1 @@ -110,7 +110,8 @@ l l. .TP .B "\-v" -Increase verbosity, including printing frequent download-progress messages. +Increase verbosity, including printing frequent download-progress messages +and printing the http response headers from the server. .SH "SECURITY" diff --git a/doc/debuginfod.8 b/doc/debuginfod.8 index f9a418d1..fde06bb8 100644 --- a/doc/debuginfod.8 +++ b/doc/debuginfod.8 @@ -258,6 +258,15 @@ Unknown buildid / request combinations result in HTTP error codes. This file service resemblance is intentional, so that an installation can take advantage of standard HTTP management infrastructure. +Upon finding a file in an archive or simply in the database, some +custom http headers are added to the response. For files in the +database X-DEBUGINFOD-FILE and X-DEBUGINFOD-SIZE are added. +X-DEBUGINFOD-FILE is simply the unescaped filename and +X-DEBUGINFOD-SIZE is the size of the file. For files found in archives, +in addition to X-DEBUGINFOD-FILE and X-DEBUGINFOD-SIZE, +X-DEBUGINFOD-ARCHIVE is added. X-DEBUGINFOD-ARCHIVE is the name of the +archive the file was found in. + There are three requests. In each case, the buildid is encoded as a lowercase hexadecimal string. For example, for a program \fI/bin/ls\fP, look at the ELF note GNU_BUILD_ID: diff --git a/tests/ChangeLog b/tests/ChangeLog index 1abe5456..23aeec4a 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -106,11 +106,12 @@ run-debuginfod-query-retry.sh, run-debuginfod-regex.sh, run-debuginfod-sizetime.sh, - run-debuginfod-tmp-home.sh, and - run-debuginfod-writable.sh - run-debuginfod-x-forwarded-for.sh - * tests/Makefile.am: Added the above new files to the test suite - * tests/test-subr.sh: Added some general functions for above tests + run-debuginfod-tmp-home.sh, + run-debuginfod-writable.sh, and + run-debuginfod-x-forwarded-for.sh. + All of the above functions now use a 'base' variable to avoid races + * Makefile.am: Added the above new files to the test suite + * test-subr.sh: Added some general functions for above tests 2021-08-28 Mark Wielaard @@ -160,6 +161,13 @@ * backtrace.c (callback_verify): Check for pthread_kill as first frame. Change asserts to fprintf plus abort. +2021-08-02 Noah Sanci + + PR27277 + * run-debuginfod-response-headers.sh: Add a test to ensure that file descriptions + are accurate for files outside or within archives. + * Makefile.am: Add run-debuginfod-response-headers.sh to TESTS. + 2021-07-26 Noah Sanci PR27982 diff --git a/tests/Makefile.am b/tests/Makefile.am index c586422e..ef097567 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -231,7 +231,8 @@ TESTS += run-debuginfod-dlopen.sh \ run-debuginfod-federation-sqlite.sh \ run-debuginfod-federation-link.sh \ run-debuginfod-federation-metrics.sh \ - run-debuginfod-x-forwarded-for.sh + run-debuginfod-x-forwarded-for.sh \ + run-debuginfod-response-headers.sh endif endif diff --git a/tests/run-debuginfod-fd-prefetch-caches.sh b/tests/run-debuginfod-fd-prefetch-caches.sh index 61fee9e9..bee88c1e 100755 --- a/tests/run-debuginfod-fd-prefetch-caches.sh +++ b/tests/run-debuginfod-fd-prefetch-caches.sh @@ -51,9 +51,9 @@ grep 'prefetch fds ' vlog$PORT1 #$PREFETCH_FDS grep 'prefetch mbs ' vlog$PORT1 #$PREFETCH_MBS # search the vlog to find what metric counts should be and check the correct metrics # were incrimented -wait_ready $PORT1 'fdcache_op_count{op="enqueue"}' $( grep -c 'interned.*front=1' vlog$PORT1 ) -wait_ready $PORT1 'fdcache_op_count{op="evict"}' $( grep -c 'evicted a=.*' vlog$PORT1 ) -wait_ready $PORT1 'fdcache_op_count{op="prefetch_enqueue"}' $( grep -c 'interned.*front=0' vlog$PORT1 ) +wait_ready $PORT1 'fdcache_op_count{op="enqueue"}' $( grep -c 'interned.*front=1' vlog$PORT1 || true) +wait_ready $PORT1 'fdcache_op_count{op="evict"}' $( grep -c 'evicted a=.*' vlog$PORT1 || true ) +wait_ready $PORT1 'fdcache_op_count{op="prefetch_enqueue"}' $( grep -c 'interned.*front=0' vlog$PORT1 || true ) wait_ready $PORT1 'fdcache_op_count{op="prefetch_evict"}' $( grep -c 'evicted from prefetch a=.*front=0' vlog$PORT1 || true ) kill $PID1 diff --git a/tests/run-debuginfod-response-headers.sh b/tests/run-debuginfod-response-headers.sh new file mode 100755 index 00000000..a458ca1b --- /dev/null +++ b/tests/run-debuginfod-response-headers.sh @@ -0,0 +1,118 @@ +#!/usr/bin/env bash +# +# Copyright (C) 2019-2021 Red Hat, Inc. +# This file is part of elfutils. +# +# This file is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# elfutils is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +. $srcdir/test-subr.sh # includes set -e + +# for test case debugging, uncomment: +set -x + +DB=${PWD}/.debuginfod_tmp.sqlite +tempfiles $DB +export DEBUGINFOD_CACHE_PATH=${PWD}/.client_cache + +# This variable is essential and ensures no time-race for claiming ports occurs +# set base to a unique multiple of 100 not used in any other 'run-debuginfod-*' test +base=9500 +get_ports +mkdir F R + +env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS= ${abs_builddir}/../debuginfod/debuginfod $VERBOSE -F -R -d $DB -p $PORT1 -t0 -g0 -v R F > vlog$PORT1 2>&1 & +PID1=$! +tempfiles vlog$PORT1 +errfiles vlog$PORT1 +# Server must become ready +wait_ready $PORT1 'ready' 1 +export DEBUGINFOD_URLS=http://127.0.0.1:$PORT1/ # or without trailing / +# Check thread comm names +ps -q $PID1 -e -L -o '%p %c %a' | grep groom +ps -q $PID1 -e -L -o '%p %c %a' | grep scan +ps -q $PID1 -e -L -o '%p %c %a' | grep traverse + +# We use -t0 and -g0 here to turn off time-based scanning & grooming. +# For testing purposes, we just sic SIGUSR1 / SIGUSR2 at the process. + +######################################################################## + +# Compile a simple program, strip its debuginfo and save the build-id. +# Also move the debuginfo into another directory so that elfutils +# cannot find it without debuginfod. +echo "int main() { return 0; }" > ${PWD}/prog.c +tempfiles prog.c +# Create a subdirectory to confound source path names +mkdir foobar +gcc -Wl,--build-id -g -o prog ${PWD}/foobar///./../prog.c +testrun ${abs_top_builddir}/src/strip -g -f prog.debug ${PWD}/prog +BUILDID=`env LD_LIBRARY_PATH=$ldpath ${abs_builddir}/../src/readelf \ + -a prog | grep 'Build ID' | cut -d ' ' -f 7` + +mv prog F +mv prog.debug F +kill -USR1 $PID1 +# Wait till both files are in the index. +wait_ready $PORT1 'thread_work_total{role="traverse"}' 1 +wait_ready $PORT1 'thread_work_pending{role="scan"}' 0 +wait_ready $PORT1 'thread_busy{role="scan"}' 0 + +cp -rvp ${abs_srcdir}/debuginfod-rpms R +if [ "$zstd" = "false" ]; then # nuke the zstd fedora 31 ones + rm -vrf R/debuginfod-rpms/fedora31 +fi + +kill -USR1 $PID1 +# Wait till both files are in the index and scan/index fully finished +wait_ready $PORT1 'thread_work_total{role="traverse"}' 2 +wait_ready $PORT1 'thread_work_pending{role="scan"}' 0 +wait_ready $PORT1 'thread_busy{role="scan"}' 0 +# All rpms need to be in the index, except the dummy permission-000 one +rpms=$(find R -name \*rpm | grep -v nothing | wc -l) +wait_ready $PORT1 'scanned_files_total{source=".rpm archive"}' $rpms +kill -USR1 $PID1 # two hits of SIGUSR1 may be needed to resolve .debug->dwz->srefs +# Wait till both files are in the index and scan/index fully finished +wait_ready $PORT1 'thread_work_total{role="traverse"}' 3 +wait_ready $PORT1 'thread_work_pending{role="scan"}' 0 +wait_ready $PORT1 'thread_busy{role="scan"}' 0 + +######################################################################## +## PR27277 +# Make a simple request to the debuginfod server and check debuginfod-find's vlog to see if +# the custom HTTP headers are received. +rm -rf $DEBUGINFOD_CACHE_PATH +env DEBUGINFOD_URLS="http://127.0.0.1:"$PORT1 LD_LIBRARY_PATH=$ldpath ${abs_top_builddir}/debuginfod/debuginfod-find\ + -vvv executable F/prog > vlog-find$PORT1.1 2>&1 +tempfiles vlog-find$PORT1.1 +grep 'Content-Length: ' vlog-find$PORT1.1 +grep 'Connection: ' vlog-find$PORT1.1 +grep 'Cache-Control: ' vlog-find$PORT1.1 +grep 'X-DEBUGINFOD-FILE: ' vlog-find$PORT1.1 +grep 'X-DEBUGINFOD-SIZE: ' vlog-find$PORT1.1 + +# Check to see if an executable file located in an archive prints the file's description and archive +env DEBUGINFOD_URLS="http://127.0.0.1:"$PORT1 LD_LIBRARY_PATH=$ldpath ${abs_top_builddir}/debuginfod/debuginfod-find\ + -vvv executable c36708a78618d597dee15d0dc989f093ca5f9120 > vlog-find$PORT1.2 2>&1 +tempfiles vlog-find$PORT1.2 +grep 'Content-Length: ' vlog-find$PORT1.2 +grep 'Connection: ' vlog-find$PORT1.2 +grep 'Cache-Control: ' vlog-find$PORT1.2 +grep 'X-DEBUGINFOD-FILE: ' vlog-find$PORT1.2 +grep 'X-DEBUGINFOD-SIZE: ' vlog-find$PORT1.2 +grep 'X-DEBUGINFOD-ARCHIVE: ' vlog-find$PORT1.2 + +kill $PID1 +wait $PID1 +PID1=0 +exit 0 diff --git a/tests/test-subr.sh b/tests/test-subr.sh index 41e95e31..95dcbb26 100644 --- a/tests/test-subr.sh +++ b/tests/test-subr.sh @@ -43,9 +43,9 @@ remove_files= exit_cleanup() { rm -rf ${PWD}/.client_cache* - rm -f $remove_files; + rm -f $remove_files; ls -a ${PWD} - cd ..; + cd ..; rmdir $test_dir } trap exit_cleanup 0 @@ -228,7 +228,7 @@ cleanup() { if [ $PID1 -ne 0 ]; then kill $PID1; wait $PID1; fi if [ $PID2 -ne 0 ]; then kill $PID2; wait $PID2; fi - rm -rf ${PWD}/.client_cache F R D L Z ${PWD}/foobar ${PWD}/mocktree + rm -rf ${PWD}/.client_cache F R D L Z ${PWD}/foobar ${PWD}/mocktree exit_cleanup } trap cleanup 0 1 2 3 5 9 15 @@ -316,12 +316,12 @@ archive_test() { get_ports() { while true; do - PORT1=`expr '(' $RANDOM % 1000 ')' + 9000` + PORT1=`expr '(' $RANDOM % 100 ')' + $base` ss -atn | fgrep ":$PORT1" || break done # Some tests will use two servers, so assign the second var while true; do - PORT2=`expr '(' $RANDOM % 1000 ')' + 9000` + PORT2=`expr '(' $RANDOM % 100 ')' + $base` ss -atn | fgrep ":$PORT2" && $PORT1 -ne $PORT2 || break done -- 2.31.1