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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
