On Wed, 2022-11-02 at 07:55 -0400, Mimi Zohar wrote: > On Wed, 2022-11-02 at 09:08 +0100, Roberto Sassu wrote: > > On Mon, 2022-10-31 at 16:25 +0000, Isaac Matthews wrote: > > > Using bpf_ima_file_hash() from kernel 6.0. > > > > > > When using bpf_ima_file_hash() with the lsm.s/file_open hook, a > > > hash > > > of the file is only sometimes returned. This is because the > > > FMODE_CAN_READ flag is set after security_file_open() is already > > > called, and ima_calc_file_hash() only checks for FMODE_READ not > > > FMODE_CAN_READ in order to decide if a new instance needs to be > > > opened. Because of this, if a file passes the FMODE_READ > > > check it > > > will fail to be hashed as FMODE_CAN_READ has not yet been set. > > > > > > To demonstrate: if the file is opened for write for example, when > > > ima_calc_file_hash() is called and the file->f_mode is checked > > > against > > > FMODE_READ, a new file instance is opened with the correct flags > > > and > > > a > > > hash is returned. If the file is opened for read, a new file > > > instance > > > is not returned in ima_calc_file_hash() as (!(file->f_mode & > > > FMODE_READ)) is now false. When __kernel_read() is called as part > > > of > > > ima_calc_file_hash_tfm() it will fail on if (!(file->f_mode & > > > FMODE_CAN_READ)) and so no hash will be returned by > > > bpf_ima_file_hash(). > > > > > > If possible could someone please advise me as to whether this is > > > intended behaviour, and is it possible to either modify the flags > > > with > > > eBPF or to open a new instance with the correct flags set as IMA > > > does > > > currently? > > > > Hi Isaac > > > > I think this is the intended behavior, as IMA is supposed to be > > called > > when the file descriptor is ready to use. > > > > If we need to call ima_file_hash() from lsm.s/file_open, I think it > > should not be a problem to create a new fd just for eBPF, in > > __ima_inode_hash(). > > > > Mimi, what do you think? > > Who/what is checking that this is a regular file and we have > permission > to open the file? Are we relying on eBPF to do this? Will opening a > file circumvent all of the LSM checks? Opening the file again will cause another permission request to be sent to LSMs, and thus to the eBPF program implementing lsm.s/file_open. Maybe it is not a good idea to use this hook. In the future, if IMA/EVM stacking is successful, we might introduce the file_post_open hook, which I believe could be suitable for calling bpf_ima_file_hash(). Roberto > > > Alternatively, would a better solution be adding a check for > > > FMODE_CAN_READ to ima_calc_file_hash()? I noticed in the comment > > > above > > > the conditional in ima_calc_file_hash() that the conditional > > > should > > > be > > > checking whether the file can be read, but only checks the > > > FMODE_READ > > > flag which is not the only requirement for __kernel_read() to be > > > able > > > to read a file. > > > > > > Thanks for your help. > > > Isaac
