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

[Mimi Zohar]

On Thu, 2022-03-03 at 17:17 +0100, KP Singh wrote:
> 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

Sounds good.

> 
> > 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).
> 
> 
> > interested in eBPF module integrity to define IMA policy rules.

That might be true for your usecase, but not Roberto's.  From the cover
letter above, Roberto was honest in saying:

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).

-- 
thanks,

Mimi