Re: [PATCH] PR28204, debuginfod IMA

["Frank Ch. Eigler" <fche@redhat.com>]

Hi, Mark -


> > Considering how easily the trybots can process the actual code - and
> > have done so before posting the patch for review - we can consider
> > some CI well done already.  After approval but before merge, it would
> > undergo another round of trybotting.  With such workflow, patchwork
> > does not need to concern itself with additional pre-commit CI/CD.
> 
> My point is really that posting with git format-patch or send-email
> makes it possible for someone to simply use git am, b4 or git pw to try
> out a patch. If the patch doesn't apply then that will be the first
> review issue.

I see what you mean, but maybe that puts the cart ahead of the horse.
What's desirable is easy access to a runnable version of the patch,
not the choice of command line tooling to do that.  A published git
branch containing the same patch is even simpler than using git
am/b4/pw.  git is the most convenient transport protocol for patches.


> I think my issue is really that it is unclear what "reassurances" we
> are making. What is the threat-model? Permissive says to me that
> although checks are done, they don't block receiving files. [...]

In the current version of the users/fche/try-bz28204 branch, this
has been renamed "optimistic", and the man page blurb now says:

   \fIima:optimistic\fP Every downloaded file with a known-invalid
   signature is rejected, protecting against some types of corruption.


> > They already make the decision whom they download debuginfo from.
> > That's literally what setting $DEBUGINFOD_URLS is.  The scenario
> > you're describing would be trusting a server enough to supply content,
> > trusting our code to fetch & check that content, but not trusting us
> > to redistribute public certificates for the servers.
> 
> The user shouldn't trust a middle-person. Unless we are signing those
> files we shouldn't distribute the certificates are being trustable
> imho.

We sign our elfutils releases, and packagers often sign their builds
of our releases, which users can verify.


> So you propose we setup a curating process to decide which certificates
> to include? 

Sure.  We already curate a set of debuginfod servers.

> If we do then I would suggest we create a separate package for that
> (or just a separate tar ball or repository). [...]

Another compromise possibility is to keep shipping these certificates,
but not include them in the default $DEBUGINFOD_IMA_CERT_PATH.  A user
wishing to trust our curation can add /etc/debuginfod/ima-certs.


> > > I do understand hex2dec, but I don't understand what extract_skid
> > > does. Maybe add an explanation what a certificates subject key id is
> > > and why we need it.

It's a brief fingerprint/hash of the key, convenient for
identification in diagnostics.


> > > [...]
> > > > Not sure, but this is how libimaevm.c similar code does it.  I assume
> > > > the first byte of the signature is something else.
> > > > (https://git.code.sf.net/p/linux-ima/ima-evm-utils)
> > > 
> > > ewww. Does this pass ubsan (--enable-sanitize-undefined)?  
> > 
> > Haven't tried but it passes valgrind.
> 
> valgrind doesn't check for undefined behavior or type alignment
> requirements.

An elfutils build with --enable-sanitize-undefined had no complaints.


> > OK, will take a look at that.  What other global-state conflicting
> > things did you notice?
> 
> A quick look at the code shows that various functions can read/write a
> static public_keys variable linked list, including (indirectly)
> ima_verify_signature. So that can cause data-races. [...]

OK, added a mutex around the entire function.


> One other issue I noticed is that it seems to be distributed under
> GPLv2-only. Which seems to mean that anything based on it should also
> be distributed under GPLv2-only, which would include libdebuginfod. Is
> there code we can rely on that is GPLv2+ and LGPLv3+ compatible?

Ugh.  I don't know of an alternative.  There isn't an equivalent
command line wrapping of the library either (with respect to
certificate searching) that we could fork out to.  (OTOH, GPLv2 is
compatible with GPLv2+.)


> > openssl's initialization is fine & thread-safe in practice, despite
> > the documentation's warnings.
> 
> OK. But even if it is thread-safe, you also need to make sure it inits
> the same. [...]

Interesting issue.  One openssl initialization call is deep inside
libcurl, another one in libimaevm's solib ctor.  Neither is
parametrized such that we could influence them.  However, things are
working(tm).


- FChE