Re: [PATCH] PR28204, debuginfod IMA

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

Hi -


> > I would not expect the emailed patch to apply, esp. with all the other
> > work done in the intermediate months, which is why the code is also in
> > the git branch.  The binary files do not seem effectively reviewable
> > anyway.
> 
> It would be really convenient though. And modern git format-patch does
> includes base tree information which allows tools to stich commits at
> the right place. 

(I would be surprised if many-month-old patches could just be
automatically "stitched".)

> It would also help with patchwork and pre-commit CI.
> https://git-scm.com/docs/git-format-patch#_base_tree_information

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.


> > > [...]
> > > >     The default is ima:permissive mode, which allows signatures to
> > > >     function like a checksum to detect accidental corruption, but accepts
> > > >     operation in a mix of signed and unsigned packages & servers.
> > > 
> > > I still think "permissive" is confusing. Since it is a term also used
> > > by e.g. selinux, but doesn't work that way. And it doesn't seem
> > > connected with the threat-model that enforcing protects against.
> > 
> > The connection is the following:
> > "enforcing" mode protects against accidental or deliberate (MITM) corruption.
> > "permissive" mode protects against accidental corruption.
> > 
> > > Since it is a different concept maybe it shouldn't be part of this
> > > patch. It is a form of integrity checking, but doesn't protect (or
> > > warns) about integrity failures. 
> > 
> > It does protect and warn against integrity failures of the form of 
> > incorrect signatures.

> My issue is that I don't really understand "permissive". Originally I
> assumed it was like selinux permissive mode, it does do the checks,
> but if they fail we just warn and continue. That seems a clear concept.

The proposed documentation explains it thusly:

  ima:enforcing Every downloaded file requires a valid signature.

  ima:permissive  Every downloaded file accompanied by a signature
  must be valid, but downloads without signatures are accepted.

  ima:ignore Skips verification altogether.

You're right that it is not an exact match for the selinux concept.
But if one's not hunting around for a precise analogy, and just reads
the single sentence description, it tries to be clear.

> [...]  if there is a signature, but we don't have the corresponding
>  certificate to check it against, should it still fail, or is it
>  more like a none-signed file and we can be "permissive" and accept
>  it? Maybe I don't have enough imagination.

I see your point.  One could make an argument either way, coming from
fuzziness with the concept of an "invalid signature".  One could
clarify with a rewording to "known-invalid".  Then "permissive"
becomes permit everything except known-invalid files.  Missing
certificates would not qualify as known-invalid, merely unknown.
Would you like me to draft up a sentence or two description of that
concept for the man page?


The intended benefit of permissive mode as a default is to give
elfutils users as much reassurance possible, without requiring manual
configuration changes or manual downloads.  See also the certificate
distribution topic below - it's really toward the same goal.


> [...]
> > Yes, it is odd.  Unfortunately, it is not possible to enforce crypto
> > signatures from distros upon unsigned slices.  A couple of possible
> > solutions:
> > [...]
> > - disable section queries from enforcing-mode servers (which could
> >   then nuke gdbindex capability for e.g. future fedora/gdb users)
> [...]
> 
> I think only option 2 makes sense given the enforcing threat-model.
> 
> Optionally we could do the section part locally, download the whole
> file, check the ima signature, then provide the application with just
> the section data.

Yeah.  That is what I was thinking, just not expressing properly.


> > > Including default system directories seems fine. But I don't think
> > > elfutils should ship certificates itself. That is the job of the
> > > distro or user.
> > 
> > The user or the distro the user is running may not be the same one
> > that the binaries the user is debugging comes from.  By shipping
> > Fedora/RHEL/CentOS certificates, we allow a Ubuntu person to debug a
> > RHEL container, and trust debuginfod content for it.
> 
> But it should be the distro/user who makes that choice. We cannot
> decide for others who they trust as provider of the files they
> download.

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.


> > > We aren't in a position to make sure the certificates are valid
> > > and/or can be trusted.
> > 
> > Why not?  We can document where we got them - I believe they are all
> > public somewhere or other already.
> 
> We certainly should document that and provide pointers to where
> distros publish their certificates. But we shouldn't install them by
> default. The distro/user can make their own choice of using them, just
> like they decide whether or not the have default DEBUGINFOD_URLS.

An elfutils-carrying distro can already decide what to do with out
certificates by including or excluding them from their package.  They
govern what's installed by default, not we.  By including the certs in
elfutils, we are making it easy for a packager to pass these on, if
they wish.

That leaves the user.  Under what conditions do you think all of the
following might hold?

- a user designates a IMA-signature-serving debuginfod as his server
- the user enabled enforcing mode
- the user does not trust our copy of the certs
- the user does not download the target distro certs for himself

I think it would take all four of those conditions for there to be a
difference between having the elfutils copy of these distro certs be
installed or not.


Another way of looking at it is to remind ourselves of the goal of
this permissive/cert-distribution default mentality: to provide
maximum possible assurance possible out of the box.  If we do not
distribute certificates, who would?  Distros may post their own on
their web sites, or include them in some packages.  But no single
distro is likely to go to extra effort to package -other distro's-
certificates.  Nor necessarily will any distro pack all their own
certificates (for versions old & new) into a standard package where
debuginfod-client can find them.  We are uniquely positioned to
"federate" this aspect.


> > > [...]
> > > It would be good to add some comments for extract_skid, I am not sure
> > > I understand how this works.
> > 
> > (Ditto.)
> 
> 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.

(I meant I'm not sure how this works either. :-) It's based on code
from imaevm.)


> [...]
> > 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.

> The issue is that this seems to access structure values at a
> possibly unaligned address.

Interesting.


> > > What does init_public_keys do? Is it thread-safe?
> > 
> > Good catch.  It initialized a global inside libimaevm.c.  It does not
> > appear thread-safe.  Will wrap this in a pthread-once or somesuch.

> libimaevm.c seems not thread-safe in general. You might have to put
> a big lock around the whole signature extraction/checking block
> which uses those library functions.

OK, will take a look at that.  What other global-state conflicting
things did you notice?


> Another possible issue might be the initialization of openssl in the
> static constructure. How does that interact with how libcurl inits
> openssl?

openssl's initialization is fine & thread-safe in practice, despite
the documentation's warnings.


- FChE