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