Re: [PATCH v3 0/9] bpf-lsm: Extend interoperability with IMA

[KP Singh]

On Thu, Mar 3, 2022 at 5:05 PM Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote:
>
> [Cc'ing Florent, Kees]
>
> Hi Alexei,
>
> On Wed, 2022-03-02 at 14:20 -0800, Alexei Starovoitov wrote:
> > On Wed, Mar 02, 2022 at 12:13:55PM +0100, Roberto Sassu wrote:
> > > Extend the interoperability with IMA, to give wider flexibility for the
> > > implementation of integrity-focused LSMs based on eBPF.
> > >
> > > Patch 1 fixes some style issues.
> > >
> > > Patches 2-6 give the ability to eBPF-based LSMs to take advantage of the
> > > measurement capability of IMA without needing to setup a policy in IMA
> > > (those LSMs might implement the policy capability themselves).
> > >
> > > Patches 7-9 allow eBPF-based LSMs to evaluate files read by the kernel.
> > >
> > > Changelog
> > >
> > > v2:
> > > - Add better description to patch 1 (suggested by Shuah)
> > > - Recalculate digest if it is not fresh (when IMA_COLLECTED flag not set)
> > > - Move declaration of bpf_ima_file_hash() at the end (suggested by
> > >   Yonghong)
> > > - Add tests to check if the digest has been recalculated
> > > - Add deny test for bpf_kernel_read_file()
> > > - Add description to tests
> > >
> > > v1:
> > > - Modify ima_file_hash() only and allow the usage of the function with the
> > >   modified behavior by eBPF-based LSMs through the new function
> > >   bpf_ima_file_hash() (suggested by Mimi)
> > > - Make bpf_lsm_kernel_read_file() sleepable so that bpf_ima_inode_hash()
> > >   and bpf_ima_file_hash() can be called inside the implementation of
> > >   eBPF-based LSMs for this hook
> > >
> > > Roberto Sassu (9):
> > >   ima: Fix documentation-related warnings in ima_main.c
> > >   ima: Always return a file measurement in ima_file_hash()
> > >   bpf-lsm: Introduce new helper bpf_ima_file_hash()
> > >   selftests/bpf: Move sample generation code to ima_test_common()
> > >   selftests/bpf: Add test for bpf_ima_file_hash()
> > >   selftests/bpf: Check if the digest is refreshed after a file write
> > >   bpf-lsm: Make bpf_lsm_kernel_read_file() as sleepable
> > >   selftests/bpf: Add test for bpf_lsm_kernel_read_file()
> > >   selftests/bpf: Check that bpf_kernel_read_file() denies reading IMA
> > >     policy
> >
> > We have to land this set through bpf-next.
> > Please get the Acks for patches 1 and 2, so we can proceed.
>

Hi Mimi,

> Each year in the LSS integrity status update talk, I've mentioned the
> eBPF integrity gaps.  I finally reached out to KP, Florent Revest, Kees

Thanks for bringing this up and it's very timely because we have been
having discussion around eBPF program signing and delineating that
from eBPF program integrity use-cases.

My plan is to travel to LSS (travel and visa permitting) and we can discuss
it more there.

If you prefer we can also discuss it before in one of the BPF office hours:

https://docs.google.com/spreadsheets/d/1LfrDXZ9-fdhvPEp_LHkxAMYyxxpwBXjywWa0AejEveU/edit#gid=0

> and others, letting them know that I'm concerned about the eBPF module
> integrity gaps.  True there is a difference between signing the eBPF
> source modules versus the eBPF generated output, but IMA could at least
> verify the integrity of the source eBPF modules making sure they are
> measured, the module hash audited, and are properly signed.
>
> Before expanding the ima_file_hash() or ima_inode_hash() usage, I'd
> appreciate someone adding the IMA support to measure, appraise, and
> audit eBPF modules.  I realize that closing the eBPF integrity gaps is
> orthogonal to this patch set, but this patch set is not only extending

This really is orthogonal and IMHO it does not seem rational to block this
patchset on it.

> the ima_file_hash()/ima_inode_hash() usage, but will be used to
> circumvent IMA.  As per usual, IMA is policy based, allowing those

I don't think they are being used to circumvent IMA but for totally
different use-cases (e.g. as a data point for detecting attacks).

- KP

> interested in eBPF module integrity to define IMA policy rules.
>
> thanks,
>
> Mimi
>