PATCH PR30962, debuginfod

PATCH PR30962, debuginfod

[Frank Ch. Eigler]

commit e967988e419121cad1d7f40013a316059b1173f0
Author: Frank Ch. Eigler <fche@redhat.com>
Date:   Tue Oct 10 16:21:00 2023 -0400

    PR30962: debuginfod: full paths for X-DEBUGINFOD-FILE/ARCHIVE response headers
    
    Previous code was inconsistent in offering basename versus full
    pathname for these headers.  The documentation was not explicit on
    this issue.  We now simplify by always passing full names back, and
    document this in the debuginfod.8 man page, along with pointers to
    how to use proxy front-end servers to strip them if needed.
    
    Signed-Off-By: Frank Ch. Eigler <fche@redhat.com>

diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index e53228803bb0..c11aeda1a3af 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -1876,11 +1876,10 @@ handle_buildid_f_match (bool internal_req_t,
     }
   else
     {
-      std::string file = b_source0.substr(b_source0.find_last_of("/")+1, b_source0.length());
       add_mhd_response_header (r, "Content-Type", "application/octet-stream");
       add_mhd_response_header (r, "X-DEBUGINFOD-SIZE",
 			       to_string(s.st_size).c_str());
-      add_mhd_response_header (r, "X-DEBUGINFOD-FILE", file.c_str());
+      add_mhd_response_header (r, "X-DEBUGINFOD-FILE", b_source0.c_str());
       add_mhd_last_modified (r, s.st_mtime);
       if (verbose > 1)
 	obatched(clog) << "serving file " << b_source0 << " section=" << section << endl;
@@ -2164,14 +2163,12 @@ handle_buildid_r_match (bool internal_req_p,
         }
       else
         {
-          std::string file = b_source1.substr(b_source1.find_last_of("/")+1, b_source1.length());
           add_mhd_response_header (r, "Content-Type",
                                    "application/octet-stream");
           add_mhd_response_header (r, "X-DEBUGINFOD-SIZE",
                                    to_string(archive_entry_size(e)).c_str());
-          add_mhd_response_header (r, "X-DEBUGINFOD-ARCHIVE",
-                                   b_source0.c_str());
-          add_mhd_response_header (r, "X-DEBUGINFOD-FILE", file.c_str());
+          add_mhd_response_header (r, "X-DEBUGINFOD-ARCHIVE", b_source0.c_str());
+          add_mhd_response_header (r, "X-DEBUGINFOD-FILE", b_source1.c_str());
           add_mhd_last_modified (r, archive_entry_mtime(e));
           if (verbose > 1)
 	    obatched(clog) << "serving archive " << b_source0
diff --git a/doc/debuginfod.8 b/doc/debuginfod.8
index d4316bec8175..7003a5823d34 100644
--- a/doc/debuginfod.8
+++ b/doc/debuginfod.8
@@ -307,11 +307,11 @@ 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-FILE is simply the full path name 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.
+X-DEBUGINFOD-ARCHIVE is added.  X-DEBUGINFOD-ARCHIVE is the full path
+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,
@@ -485,8 +485,9 @@ a denial-of-service in terms of RAM, CPU, disk I/O, or network I/O.
 If this is a problem, users are advised to install debuginfod with a
 HTTPS reverse-proxy front-end that enforces site policies for
 firewalling, authentication, integrity, authorization, and load
-control.  The \fI/metrics\fP webapi endpoint is probably not
-appropriate for disclosure to the public.
+control.  Front-end proxies can also elide sensitive path name
+components in X-DEBUGINFOD-FILE/ARCHIVE response headers,
+for example using Apache httpd's \fBmod_header\fP "Header edit".
 
 When relaying queries to upstream debuginfods, debuginfod \fBdoes not\fP
 include any particular security features.  It trusts that the binaries
diff --git a/tests/run-debuginfod-response-headers.sh b/tests/run-debuginfod-response-headers.sh
index 8cb7b843d19d..fbb6a4842fa4 100755
--- a/tests/run-debuginfod-response-headers.sh
+++ b/tests/run-debuginfod-response-headers.sh
@@ -78,8 +78,8 @@ tempfiles vlog-find$PORT1.1
 errfiles vlog-find$PORT1.1
 cat vlog-find$PORT1.1
 grep 'Headers:' vlog-find$PORT1.1
-grep -i 'X-DEBUGINFOD-FILE: prog' vlog-find$PORT1.1
-grep -i 'X-DEBUGINFOD-SIZE: '     vlog-find$PORT1.1
+grep -i 'X-DEBUGINFOD-FILE: .*/prog' vlog-find$PORT1.1
+grep -i '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\
@@ -88,9 +88,9 @@ tempfiles vlog-find$PORT1.2
 errfiles vlog-find$PORT1.2
 cat vlog-find$PORT1.2
 grep 'Headers:'               vlog-find$PORT1.2
-grep -i 'X-DEBUGINFOD-FILE: '    vlog-find$PORT1.2
+grep -i 'X-DEBUGINFOD-FILE: .*/.*'    vlog-find$PORT1.2
 grep -i 'X-DEBUGINFOD-SIZE: '    vlog-find$PORT1.2
-grep -i 'X-DEBUGINFOD-ARCHIVE: ' vlog-find$PORT1.2
+grep -i 'X-DEBUGINFOD-ARCHIVE: .*/.*' vlog-find$PORT1.2
 
 # Check that X-DEBUGINFOD-SIZE matches the size of each file
 for file in vlog-find$PORT1.1 vlog-find$PORT1.2

Re: PATCH PR30962, debuginfod

[Mark Wielaard]

Hi Frank,

On Tue, 2023-10-10 at 16:37 -0400, Frank Ch. Eigler wrote:
> commit e967988e419121cad1d7f40013a316059b1173f0
> Author: Frank Ch. Eigler <fche@redhat.com>
> Date:   Tue Oct 10 16:21:00 2023 -0400
> 
>     PR30962: debuginfod: full paths for X-DEBUGINFOD-FILE/ARCHIVE response headers
>     
>     Previous code was inconsistent in offering basename versus full
>     pathname for these headers.  The documentation was not explicit on
>     this issue.  We now simplify by always passing full names back, and
>     document this in the debuginfod.8 man page, along with pointers to
>     how to use proxy front-end servers to strip them if needed.

I think this makes sense, but it would be good to see an example of the
paths this now exposes. Does this include the temporary dir that a file
is extracted in? Does it really make sense to provide the full
(absolute?) path of the archive a source file was found in?

Cheers,

Mark

Re: PATCH PR30962, debuginfod

[Frank Ch. Eigler]

Hi -

> I think this makes sense, but it would be good to see an example of the
> paths this now exposes.

e.g:
% debuginfod-find -v debuginfo /bin/ls
[...]
x-debuginfod-size: 502024
x-debuginfod-archive: /mnt/fedora_koji_prod/koji/packages/coreutils/9.3/4.fc39/x86_64/coreutils-debuginfo-9.3-4.fc39.x86_64.rpm
x-debuginfod-file: /usr/lib/debug/usr/bin/ls-9.3-4.fc39.x86_64.debug
[...]

> Does this include the temporary dir that a file is extracted in?

No.

> Does it really make sense to provide the full (absolute?) path of
> the archive a source file was found in?

As much sense as omitting the entire path information and returning
only the basename.  Sometimes the path may matter, if e.g. the archive
file names are duplicate.  And it turns out the archive paths were
already usually sent in their entirety, and the archived-file paths
were basename'd.  Kinda the wrong way around.  Anyway it's simplest to
do neither bit of elision at the tool level.

- FChE

Re: PATCH PR30962, debuginfod

[Mark Wielaard]

Hi Frank,

On Wed, 2023-10-11 at 10:22 -0400, Frank Ch. Eigler wrote:
> > I think this makes sense, but it would be good to see an example of the
> > paths this now exposes.
> 
> e.g:
> % debuginfod-find -v debuginfo /bin/ls
> [...]
> x-debuginfod-size: 502024
> x-debuginfod-archive: /mnt/fedora_koji_prod/koji/packages/coreutils/9.3/4.fc39/x86_64/coreutils-debuginfo-9.3-4.fc39.x86_64.rpm
> x-debuginfod-file: /usr/lib/debug/usr/bin/ls-9.3-4.fc39.x86_64.debug
> [...]

Ah, right. Thanks.

> > Does this include the temporary dir that a file is extracted in?
> 
> No.
> 
> > Does it really make sense to provide the full (absolute?) path of
> > the archive a source file was found in?
> 
> As much sense as omitting the entire path information and returning
> only the basename.  Sometimes the path may matter, if e.g. the archive
> file names are duplicate.  And it turns out the archive paths were
> already usually sent in their entirety, and the archived-file paths
> were basename'd.  Kinda the wrong way around.  Anyway it's simplest to
> do neither bit of elision at the tool level.

OK. But I think you should add an explanation or example to "Front-end
proxies can also elide sensitive path name components" paragraph. So
the user is fully aware what those "sensitive path names" are. Maybe
even add that debuginfod-find -v example so people can double check.

Thanks,

Mark

Re: PATCH PR30962, debuginfod

[Frank Ch. Eigler]

Hi -

> OK. But I think you should add an explanation or example to "Front-end
> proxies can also elide sensitive path name components" paragraph. So
> the user is fully aware what those "sensitive path names" are. Maybe
> even add that debuginfod-find -v example so people can double check.

OK, rewrote the related text in the man page:

[...WEBAPI...]

For most queries, some custom http headers are added to the response,
providing additional metadata about the buildid-related response.  For example:

.SAMPLE
% debuginfod-find -v debuginfo /bin/ls |& grep -i x-debuginfo
x-debuginfod-size: 502024
x-debuginfod-archive: /mnt/fedora_koji_prod/koji/packages/coreutils/9.3/4.fc39/x86_64/coreutils-debuginfo-9.3-4.fc39.x86_64.rpm
x-debuginfod-file: /usr/lib/debug/usr/bin/ls-9.3-4.fc39.x86_64.debug
.ESAMPLE

.TP
X-DEBUGINFOD-SIZE
The size of the file, in bytes.  This may differ from the http Content-Length:
field (if present), due to compression in transit.

.TP
X-DEBUGINFOD-FILE
The full path name of the file related to the given buildid.

.TP
X-DEBUGINFOD-ARCHIVE
The full path name of the archive that contained the above file, if any.

[...SECURITY...]

Front-end proxies may elide sensitive path name components in
X-DEBUGINFOD-FILE/ARCHIVE response headers.  For example, using Apache
httpd's \fBmod_headers\fP, you can remove the entire directory name
prefix:

.SAMPLE
Header edit x-debuginfod-archive ".*/" ""
.ESAMPLE


- FChE






- FChE

Re: PATCH PR30962, debuginfod

[Mark Wielaard]

Hi Frank,

On Wed, 2023-10-11 at 14:57 -0400, Frank Ch. Eigler wrote:
> > OK. But I think you should add an explanation or example to "Front-end
> > proxies can also elide sensitive path name components" paragraph. So
> > the user is fully aware what those "sensitive path names" are. Maybe
> > even add that debuginfod-find -v example so people can double check.
> 
> OK, rewrote the related text in the man page:

Looks really good and informative.

Thanks,

Mark