On Thu, Mar 3, 2022 at 5:30 PM Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote: > > 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). Currently to use the helper bpf_ima_inode_hash in LSM progs one needs to have a basic IMA policy in place just to get a hash, even if one does not need to use the whole feature-set provided by IMA. These patches would be quite beneficial for this user-journey. Even Robert's use case is to implement IMA policies in BPF this is still fundamentally different from IMA doing integrity measurement for BPF and blocking this patch-set on the latter does not seem rational and I don't see how implementing integrity for BPF would avoid your concerns. > > -- > thanks, > > Mimi >