On 12/08/15 at 02:15pm, Mimi Zohar wrote: > On Tue, 2015-12-08 at 13:32 -0500, Vivek Goyal wrote: > > On Tue, Dec 08, 2015 at 01:01:21PM -0500, Mimi Zohar wrote: > > > > [..] > > > #ifdef CONFIG_IMA_APPRAISE > > > diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c > > > index b70ada0..18c4a84 100644 > > > --- a/kernel/kexec_file.c > > > +++ b/kernel/kexec_file.c > > > @@ -18,6 +18,7 @@ > > > #include <linux/kexec.h> > > > #include <linux/mutex.h> > > > #include <linux/list.h> > > > +#include <linux/ima.h> > > > #include <crypto/hash.h> > > > #include <crypto/sha.h> > > > #include <linux/syscalls.h> > > > @@ -33,7 +34,8 @@ size_t __weak kexec_purgatory_size = 0; > > > > > > static int kexec_calculate_store_digests(struct kimage *image); > > > > > > -static int copy_file_from_fd(int fd, void **buf, unsigned long *buf_len) > > > +static int copy_file_from_fd(int fd, enum ima_read_hooks read_func, > > > + void **buf, unsigned long *buf_len) > > > { > > > struct fd f = fdget(fd); > > > int ret; > > > @@ -65,14 +67,17 @@ static int copy_file_from_fd(int fd, void **buf, > > > unsigned long *buf_len) > > > goto out; > > > } > > > > > > + ret = ima_read_and_process_file(f.file, read_func, *buf, stat.size); > > > + if (ret != -EOPNOTSUPP) > > > + goto out_free; > > > + > > > > Hi Mimi, > > > > I am wondering why are we designing this function to also read the file. > > > Looks like we just want an hook which calls into ima so that ima can > > apply its *additional* policies on kernel and initramfs. If caller is > > allocating the buffer, then caller can continue to read the file as well. > > In fact that simplifies the code. Now caller which need to check > > -EOPNOTSUPP and continue to read the file anyway. > > IMA is calculating the file hash as it reads the file. The file hash is > then used for normal IMA processing - adding the measurement to the > measurement list and verifying the file's integrity. > > > IOW, if caller still has to maintain the code to read the file, then it > > is better that ima exports a hook which callers calls after reading the > > file and ima can do its own verification. > > There's no sense in reading the file twice. The file hash should be > calculated as the file is being read. Either the existing function to > read the file needs to support calculating the file hash or it should > call IMA.
Can IMA provide a function to calculate the hash from a buffer? > > There's a lot of code duplication for reading a file by the kernel (eg. > kexec, firmware, kernel modules, ...). Each place does it just a bit > differently than the other. There should be a single function for > reading and calculating the file hash at the same time. If you want to address the duplication for reading file, IMHO you can introduce a general interface to read file in kernel space. But I do not think the reading + calculating only interface is a good way. > > > Also why do we call second parameter as "read_func". I am really not > > passing a function read function. May be something lile file_type might > > be better. > > The read_func identifies the caller of ima_read_and_process_file(). > The IMA policy would then look like: > > measure func=KEXEC_CHECK > appraise func=KEXEC_CHECK appraise_type=imasig > # > measure func=INITRAMFS_CHECK > appraise func=INITRAMFS_CHECK appraise_type=imasig > # > measure func=FIRMWARE_CHECK > appraise func=FIRMWARE_CHECK appraise_type=imasig > # > measure func=POLICY_CHECK > appraise func=POLICY_CHECK appraise_type=imasig > > > So how does this additional IMA specific policies help (on top of existing > > signature verification mechanism we have for kernel). > > The existing kexec signature verification is limited to verifying the > kexec image on x86_64. It does not verify the kexec image on other > architectures, nor does it verify the initramfs. The original method > for verifying a files' integrity was IMA. We have need for verifying > both the kexec image and initramfs. > > Mimi > Thanks Dave -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html