Re: [PATCH] Fix thread-safety for elfutils
Hi John, Hi Heather, On Mon, 2023-08-21 at 17:08 -0500, John Mellor-Crummey via Elfutils- devel wrote: > Any thoughts about the patch from my student, Heather McIntyre? Apologies for not responding sooner. The patch was posted when I was on vacation and I still haven't fully caught up with all work. But I certainly haven't forgotten. I will try to look at it soon since it would be exciting to finally be able to enable thread safety. Cheers, Mark
[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives
https://sourceware.org/bugzilla/show_bug.cgi?id=28204 --- Comment #22 from Mark Wielaard --- (In reply to Ryan Goldberg from comment #21) > (In reply to Mark Wielaard from comment #20) > > But isn't the idea of checking the IMA signatures that you don't have to > > trust the server providing the debuginfo files as the distro intended them? > But this will allow for the case of a trusted server which only has some of > it's RPMs per-file signed. Take for instance a server which has the RPMs for > f36,37,38. The f36 files don't have signatures so using enforcing here is > too strict since we are ok just letting a client know that these ones are > unverifiable, but we still want to be able to reject any of the invalid ones > for f38 This still feels odd. Since you cannot distinguish between non-sig f36 package (okish?) and non-sig f38 packages (bad?). I think you have to either trust or reject all non-sig packages from such a server. > > So both are bad in some way. Which imho means that if we support some kind > > of permissive mode, then it should explicitly warn for both kind of > > baddness. > In the permissive mode you'll get: > * "the signature is valid" for valid sigs > * "ALERT: this download is being rejected since the IMA signature could not > be verified" for invalid sigs > * "the signature could not be verified" otherwise I'll look at the code to see if I understand what this means in practice. Are those the messages presented to the user (in verbose mode? always?). And does this mean all three cases warn (or only the second and third)? And is it just a message or does it also mean actual rejection in some cases? > So we do warn for both kinds of bad, we just don't reject the 'unknown' bad But how is 'unknown' bad different from invalid signature bad? I think you should assume that if there is no signature, then the signature is by definition invalid (assume it has a signature of ). -- You are receiving this mail because: You are on the CC list for the bug.
[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives
https://sourceware.org/bugzilla/show_bug.cgi?id=28204
--- Comment #23 from Ryan Goldberg ---
(In reply to Mark Wielaard from comment #22)
> This still feels odd. Since you cannot distinguish between non-sig f36
> package (okish?) and non-sig f38 packages (bad?). I think you have to either
> trust or reject all non-sig packages from such a server.
I see where you're coming from but what policy would you use for
https://debuginfod.fedoraproject.org? We'd have to ignore then, which seems
like one of the primary users of such verification might just skip it all
together.
> I'll look at the code to see if I understand what this means in practice.
> Are those the messages presented to the user (in verbose mode? always?). And
> does this mean all three cases warn (or only the second and third)? And is
> it just a message or does it also mean actual rejection in some cases?
2008 if(NULL != url_ima_policies && ignore != url_ima_policies[committed_to])
2009 {
2010 int result = debuginfod_validate_imasig(c, target_cache_tmppath, fd);
2011 if(0 == result)
2012 {
2013 if (vfd >= 0) dprintf (vfd, "the signature is valid\n");
2014 }
2015 else if(EINVAL == result || enforcing ==
url_ima_policies[committed_to])
2016 {
2017 // All invalid signatures are rejected.
2018 // Additionally in enforcing mode any non-valid signature is
rejected, so by reaching
2019 // this case we do so since we know it is not valid. Note - this not
just invalid signatures
2020 // but also signatures that cannot be validated
2021 if (vfd >= 0) dprintf (vfd, "ALERT: this download is being rejected
since the IMA signature could not be verified\n");
2022 rc = -EPERM;
2023 goto out2;
2024 }
2025 else
2026 {
2027 // By default we are permissive, so since the signature isn't
invalid we
2028 // give it the benefit of the doubt
2029 if (vfd >= 0) dprintf (vfd, "the signature could not be
verified\n");
2030 }
2031 }
2032
Here is the relevant snippet
--
You are receiving this mail because:
You are on the CC list for the bug.
[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives
https://sourceware.org/bugzilla/show_bug.cgi?id=28204 --- Comment #24 from Mark Wielaard --- BTW. How does this interact with the "section" queries? If the server doesn't support "section" then the debuginfod-client fallback to fetching "debuginfo" or "executable" should do the signature checking. But there doesn't seem to be a way to make (client side) signature checking work when fetching "section". Does this mean that in enforcing mode we always need to use the fallback mode? -- You are receiving this mail because: You are on the CC list for the bug.
[Bug debuginfod/28204] extend webapi / verification with forthcoming signed-contents archives
https://sourceware.org/bugzilla/show_bug.cgi?id=28204 --- Comment #25 from Mark Wielaard --- So I am looking at users/rgoldber/try-bz28204d but it isn't clear you want to merge that in separate commits or squashed together. I am comparing to users/rgoldber/try-bz28204c which I believe is the previous version reviewed. It really makes things a lot easier if the actual patches that are intended to be merged are posted (with a description of what was changed since the last review). So as far as I can tell the new series has been rebased from commit 35e059b654224b1a01d05877b13582c74c692388 to 27a84961f7a061b83f10f7e02bf433c229d6537a. Good. That is just 3 commits. - configure.ac checks updated. - debuginfod/debuginfod-client.c introduces ima_policy_t Includes an "undefined" policy? debuginfod_validate_imasig updated to read/digest in chunks of DATA_SIZE. Is the k += DATA_SIZE correct? Can't pread return an n < DATA_SIZE? If the cert_paths = strdup (...) fails cert_paths gets assigned a static string? Won't that crash the strtok calls or the free (cert_path) call? In debuginfod_query_server the server_urls are parsed to see te ima policy, as described debuginfod-client-config.7 Sorry, have to stop for a bit. Will try to look at the rest later. -- You are receiving this mail because: You are on the CC list for the bug.
