Re: [PATCH] RFC: come up with startswith function.
Hi Martin,
On Mon, 2021-04-19 at 15:18 +0200, Martin Liška wrote:
> I made similar changes to binutils some time ago and I would like to
> come up with the same function for elfutils. Note that current
> construct
> is quite error prone, I found for instance these 2 bad usages:
>
> diff --git a/libdwfl/relocate.c b/libdwfl/relocate.c
> index 88b5211d..b6de3510 100644
> --- a/libdwfl/relocate.c
> +++ b/libdwfl/relocate.c
> @@ -518,7 +518,7 @@ relocate_section (Dwfl_Module *mod, Elf
> *relocated, const GElf_Ehdr *ehdr,
> Nothing to do here. */
> return DWFL_E_NOERROR;
>
> - if (strncmp (tname, ".zdebug", strlen ("zdebug")) == 0)
> + if (strncmp (tname, ".zdebug", strlen (".zdebug")) == 0)
> elf_compress_gnu (tscn, 0, 0);
>
>if ((tshdr->sh_flags & SHF_COMPRESSED) != 0)
> @@ -539,7 +539,7 @@ relocate_section (Dwfl_Module *mod, Elf
> *relocated, const GElf_Ehdr *ehdr,
>if (sname == NULL)
> return DWFL_E_LIBELF;
>
> - if (strncmp (sname, ".zdebug", strlen ("zdebug")) == 0)
> + if (strncmp (sname, ".zdebug", strlen (".zdebug")) == 0)
> elf_compress_gnu (scn, 0, 0);
>
>if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
Urgh. Thanks for finding this!
> I'm not convinced about function declaration in system.h. Is it a
> proper location?
Yes, I think it is.
> And the function is not used in debuginfod/debuginfod-client.c and
> debuginfod/debuginfod.cxx.
> I would need another decl for these, am I right?
I think they both can simply include system.h (might want to double
check the -I search path in debuginfod/Makefile.am) because system.h
should be stand-alone, you don't need to link to anything else.
Maybe for debuginfod.cxx there is a better C++ way for strings. But if
it uses C strings, then it could also simply include system.h.
Thanks,
Mark
Re: [PATCH] RFC: come up with startswith function.
Em 20/04/2021 08:43, Mark Wielaard escreveu:
Hi Martin,
On Mon, 2021-04-19 at 15:18 +0200, Martin Liška wrote:
I made similar changes to binutils some time ago and I would like to
come up with the same function for elfutils. Note that current
construct
is quite error prone, I found for instance these 2 bad usages:
diff --git a/libdwfl/relocate.c b/libdwfl/relocate.c
index 88b5211d..b6de3510 100644
--- a/libdwfl/relocate.c
+++ b/libdwfl/relocate.c
@@ -518,7 +518,7 @@ relocate_section (Dwfl_Module *mod, Elf
*relocated, const GElf_Ehdr *ehdr,
Nothing to do here. */
return DWFL_E_NOERROR;
- if (strncmp (tname, ".zdebug", strlen ("zdebug")) == 0)
+ if (strncmp (tname, ".zdebug", strlen (".zdebug")) == 0)
elf_compress_gnu (tscn, 0, 0);
if ((tshdr->sh_flags & SHF_COMPRESSED) != 0)
@@ -539,7 +539,7 @@ relocate_section (Dwfl_Module *mod, Elf
*relocated, const GElf_Ehdr *ehdr,
if (sname == NULL)
return DWFL_E_LIBELF;
- if (strncmp (sname, ".zdebug", strlen ("zdebug")) == 0)
+ if (strncmp (sname, ".zdebug", strlen (".zdebug")) == 0)
elf_compress_gnu (scn, 0, 0);
if ((shdr->sh_flags & SHF_COMPRESSED) != 0)
Urgh. Thanks for finding this!
I'm not convinced about function declaration in system.h. Is it a
proper location?
Yes, I think it is.
And the function is not used in debuginfod/debuginfod-client.c and
debuginfod/debuginfod.cxx.
I would need another decl for these, am I right?
I think they both can simply include system.h (might want to double
check the -I search path in debuginfod/Makefile.am) because system.h
should be stand-alone, you don't need to link to anything else.
Maybe for debuginfod.cxx there is a better C++ way for strings. But if
it uses C strings, then it could also simply include system.h.
std::basic_string_view::starts_with is only C++20, so I'd suggest
wrapping `startswith` to not have to type `.c_str()` all the time, but
nothing beyond that.
Thanks,
Mark
[Bug debuginfod/27758] New: security idea: DEBUGINFOD_VERIFY mode
https://sourceware.org/bugzilla/show_bug.cgi?id=27758 Bug ID: 27758 Summary: security idea: DEBUGINFOD_VERIFY mode Product: elfutils Version: unspecified Status: NEW Severity: normal Priority: P2 Component: debuginfod Assignee: unassigned at sourceware dot org Reporter: fche at redhat dot com CC: elfutils-devel at sourceware dot org Target Milestone: --- Reasonable concerns have been raised about whether a debuginfod client has any way of verifying that artifacts downloaded are unmodified / still trustworthy. This is a good question because any package-level signature protection is stripped at the server, when we serve constituent files in isolation. As transport over HTTPS protects the content, we can safely assume that immediately during/after the download, the content will be fine. However, what of cached files? What if some program changes the cache contents sometime between download and a much later reuse? (Note that this threat model is not that serious, since any tool that could modify cache contents can probably also modify dot files etc., and take over the user's account.) But anyway, as a trust/comfort measure, we could provide limited verification of cached content, without having to fully download again. Here's one possible way: - all this being conditional on a client-side environment variable like $DEBUGINFOD_VERIFY being set - in the -client.c code, during a find operation, if there is a cache hit, the client will STILL make a connection to the upstream $DEBUGINFOD_URLS, but only with a HEAD query, otherwise same webapi - the server code, upon seeing the HEAD query, will return additional response headers - one of these response headers will be X-Debuginfod-Hash: XYZXYZXYZ, which would be some securish hash of the content, probably sha256 or such - the server will compute / cache this hash in a new sqlite table, akin to the buildids9_file_mtime_scanned, for each file over time, subject to grooming as usual - how federated servers do this w.r.t serving from their own cache: TBD - the client will look for this response header from all servers that return 200 - if no server returns this header (maybe because it's just old, or they don't happen to have the hash cached or such), and if $DEBUGINFOD_VERIFY value is "permissive", result -> PASS, return - the client will pick ANY or ALL (maybe depending on bug #25607 policy?) - the client will compute the same hash function on the cached content, and compare - if the local hash mismatches the server-provided hash, warn via $DEBUGINFOD_VERBOSE, delete local cached object, perform full download - otherwise: result -> PASS, return In the elfutils profile.d/debuginfod.* files, distro policy could set $DEBUGINFOD_VERIFY=enforcing or =permissive or (none) differently for root and/or less privileged users. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug debuginfod/27758] security idea: DEBUGINFOD_VERIFY mode
https://sourceware.org/bugzilla/show_bug.cgi?id=27758 Zbigniew Jędrzejewski-Szmek changed: What|Removed |Added CC||zbyszek at in dot waw.pl --- Comment #1 from Zbigniew Jędrzejewski-Szmek --- I don't think this protection is particularly interesting. Normal file system permissions should be used to safeguard any files in the home directory. I don't see a reason to try to add a verification layer on top. And if it was added, it cannot be effective anyway. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug debuginfod/27758] security idea: DEBUGINFOD_VERIFY mode
https://sourceware.org/bugzilla/show_bug.cgi?id=27758 --- Comment #2 from Frank Ch. Eigler --- Yeah. It may comfort those who are worried about the integrity of their previously downloaded cached files, but is not robust against local attacker who currently has control over the filesystem or processes. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug debuginfod/27758] security idea: DEBUGINFOD_VERIFY mode
https://sourceware.org/bugzilla/show_bug.cgi?id=27758 Vitaly Chikunov changed: What|Removed |Added CC||vt at altlinux dot org --- Comment #3 from Vitaly Chikunov --- Instead of `X-Debuginfod-Hash` you can use `ETag` where you can put anything including sha256 (can be prescribed in webapi description), then GET request with `If-None-Match` + tag value (which is a hash) will return just 304 if the hash is not changed. So HEAD request is not needed too. https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match -- You are receiving this mail because: You are on the CC list for the bug.
