Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs

[Dave Young]

Hi, Mimi

On 12/28/15 at 07:51am, Mimi Zohar wrote:
> On Mon, 2015-12-28 at 10:08 +0800, Dave Young wrote:
> > On 12/25/15 at 09:45am, Mimi Zohar wrote:
> > > IMA calculates the file hash, in this case, based on the buffer
> > > contents.   The hash is calculated once and used for both measurement
> > > and appraisal.  If the file integrity appraisal fails (eg. hash
> > > comparison or signature failure), IMA prevents the kexec files from
> > > being used.
> > > 
> > 
> > Ok, thanks for the explanatioin. But I have another question, why do we
> > need a special hook for KEXEC? Shouldn't all files use same way to do the
> > measurement and appraisal?
> 
> "By all files" are you referring to all files read by the kernel or all
> files opened, executed or mmapped by the system?

Hmm, I means any kind of files read by the kernel.

> 
> Currently IMA allocates a page sized buffer, reads a file a page chunk
> at a time calculating the file hash as it does so, and then frees the
> buffer before returning to the caller.  This method of calculating the
> file hash is used for measuring and appraising files opened
> (FILE_CHECK), executed (BPRM_CHECK) or mmapped (MMAP_CHECK) by the
> system.
> 
> This patch set addresses files being read by kernel.  A single new
> generic hook named ima_hash_and_process_file() is defined to not only
> measure and appraise the kexec image and initramfs, but firmware and the
> IMA policy.   As we identify other places that the kernel is reading
> files, this hook would be called in those places as well.

What I can not understand is why IMA need know the caller information and
why cann't introduce a generic interface. kexec and firmware and other
caller all read files, so a common file based interface should be better?

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

Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs

[Mimi Zohar]

On Tue, 2015-12-29 at 07:06 -0500, Mimi Zohar wrote:
> On Tue, 2015-12-29 at 16:21 +0800, Dave Young wrote:

> This policy flexibility is needed at least until all files come from
> software providers with file signatures.  (RPM has been modified to
> include file signatures.)  Even then, in terms of kexec, some distros
> generate the initramfs on the target host and,  therefore, can not sign
> the initramfs.  The local user could, however, sign the initramfs on
> their own system.

Sorry, instead of "local user" the "local system/host owner" would be
more appropriate.

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

Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs

[Mimi Zohar]

On Tue, 2015-12-29 at 16:21 +0800, Dave Young wrote:
> Hi, Mimi
> 
> On 12/28/15 at 07:51am, Mimi Zohar wrote:
> > On Mon, 2015-12-28 at 10:08 +0800, Dave Young wrote:
> > > On 12/25/15 at 09:45am, Mimi Zohar wrote:
> > > > IMA calculates the file hash, in this case, based on the buffer
> > > > contents.   The hash is calculated once and used for both measurement
> > > > and appraisal.  If the file integrity appraisal fails (eg. hash
> > > > comparison or signature failure), IMA prevents the kexec files from
> > > > being used.
> > > > 
> > > 
> > > Ok, thanks for the explanatioin. But I have another question, why do we
> > > need a special hook for KEXEC? Shouldn't all files use same way to do the
> > > measurement and appraisal?
> > 
> > "By all files" are you referring to all files read by the kernel or all
> > files opened, executed or mmapped by the system?
> 
> Hmm, I means any kind of files read by the kernel.
> 
> > 
> > Currently IMA allocates a page sized buffer, reads a file a page chunk
> > at a time calculating the file hash as it does so, and then frees the
> > buffer before returning to the caller.  This method of calculating the
> > file hash is used for measuring and appraising files opened
> > (FILE_CHECK), executed (BPRM_CHECK) or mmapped (MMAP_CHECK) by the
> > system.
> > 
> > This patch set addresses files being read by kernel.  A single new
> > generic hook named ima_hash_and_process_file() is defined to not only
> > measure and appraise the kexec image and initramfs, but firmware and the
> > IMA policy.   As we identify other places that the kernel is reading
> > files, this hook would be called in those places as well.
> 
> What I can not understand is why IMA need know the caller information and
> why cann't introduce a generic interface. kexec and firmware and other
> caller all read files, so a common file based interface should be better?

The next patch set will define a common function for reading files by
the kernel.  Luis set up a wiki
http://kernelnewbies.org/KernelProjects/common-kernel-loader with some
details.

This patch set defines a generic interface for measuring and appraising
files being read by the kernel, with the ability to define a policy
based on the caller information.   For the details on expressing a
policy, refer to Documentation/ABI/testing/ima-policy.   For example,
the new rules could be expressed 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

This policy flexibility is needed at least until all files come from
software providers with file signatures.  (RPM has been modified to
include file signatures.)  Even then, in terms of kexec, some distros
generate the initramfs on the target host and,  therefore, can not sign
the initramfs.  The local user could, however, sign the initramfs on
their own system.

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

Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs

[Mimi Zohar]

On Mon, 2015-12-28 at 10:08 +0800, Dave Young wrote:
> On 12/25/15 at 09:45am, Mimi Zohar wrote:
> > IMA calculates the file hash, in this case, based on the buffer
> > contents.   The hash is calculated once and used for both measurement
> > and appraisal.  If the file integrity appraisal fails (eg. hash
> > comparison or signature failure), IMA prevents the kexec files from
> > being used.
> > 
> 
> Ok, thanks for the explanatioin. But I have another question, why do we
> need a special hook for KEXEC? Shouldn't all files use same way to do the
> measurement and appraisal?

"By all files" are you referring to all files read by the kernel or all
files opened, executed or mmapped by the system?

Currently IMA allocates a page sized buffer, reads a file a page chunk
at a time calculating the file hash as it does so, and then frees the
buffer before returning to the caller.  This method of calculating the
file hash is used for measuring and appraising files opened
(FILE_CHECK), executed (BPRM_CHECK) or mmapped (MMAP_CHECK) by the
system.

This patch set addresses files being read by kernel.  A single new
generic hook named ima_hash_and_process_file() is defined to not only
measure and appraise the kexec image and initramfs, but firmware and the
IMA policy.   As we identify other places that the kernel is reading
files, this hook would be called in those places as well.

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

Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs

[Mimi Zohar]

On Mon, 2015-12-28 at 10:08 +0800, Dave Young wrote:
> On 12/25/15 at 09:45am, Mimi Zohar wrote:
> > IMA calculates the file hash, in this case, based on the buffer
> > contents.   The hash is calculated once and used for both measurement
> > and appraisal.  If the file integrity appraisal fails (eg. hash
> > comparison or signature failure), IMA prevents the kexec files from
> > being used.
> > 
> 
> Ok, thanks for the explanatioin. But I have another question, why do we
> need a special hook for KEXEC? Shouldn't all files use same way to do the
> measurement and appraisal?

"By all files" are you referring to all files read by the kernel or all
files opened, executed or mmapped by the system?

Currently IMA allocates a page sized buffer, reads a file a page chunk
at a time calculating the file hash as it does so, and then frees the
buffer before returning to the caller.  This method of calculating the
file hash is used for measuring and appraising files opened
(FILE_CHECK), executed (BPRM_CHECK) or mmapped (MMAP_CHECK) by the
system.

This patch set addresses files being read by kernel.  A single new
generic hook named ima_hash_and_process_file() is defined to not only
measure and appraise the kexec image and initramfs, but firmware and the
IMA policy.   As we identify other places that the kernel is reading
files, this hook would be called in those places as well.

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

Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs

[Mimi Zohar]

On Fri, 2015-12-25 at 13:33 +0800, Dave Young wrote:
> Hi, Mimi
> 
> CCing kexec list, not all kexec people subscribed to IMA list.
> I just subscribed to it since Vivek CCed me last time about the V1 of this
> series.

Thanks!

> On 12/23/15 at 06:55pm, Mimi Zohar wrote:
> > This patch defines a new IMA hook ima_hash_and_process_file() for
> > measuring and appraising files read by the kernel.  The caller loads
> > the file into memory before calling this function, which calculates
> > the hash followed by the normal IMA policy based processing.
> > 
> > Two new IMA policy functions named KEXEC_CHECK and INITRAMFS_CHECK
> > are defined for measuring, appraising or auditing the kexec image
> > and initramfs.
> 
> Could you help us understand why do we need it first.

IMA can be viewed as extending secure and trusted boot to the running
system in a uniform and consistent manner.   As files are accessed,
based on policy, IMA measures them, appends the file measurements to the
running measurement list (/ima/ascii_runtime_measurements)
and appraises the file's integrity, based on either the file's hash or
signature, which are stored as extended attributes in "security.ima".

There are still a couple of file measurement and appraisal gaps that
need to be closed.

> I think I do not really understand the purpose of the IMA handling
> about kexec kernel and initramfs.

One of those measurement and appraisal gaps are files that are read by
the kernel, like the kexec image and initramfs.

[There is a lot of code duplication in the kernel for reading a file and
verifying its signature.   Each place does it just a bit differently
than the other.  I'm working with Luis Rodriguez on defining a single,
common function  - https://lkml.org/lkml/2015/12/21/478.]

> * Does the files in disk space have already contains some hash values 
> and when kernel load it IMA functions will do some checking? But seems I do 
> not
> see such handling..

IMA sits on a number of the LSM hooks, where they exist, and in other
places defines its own hook.   This patch set defines a new IMA hook for
measuring and appraising files being read by the kernel.

> * Does it try to calculate the hash of the file buffer after copying,

IMA calculates the file hash, in this case, based on the buffer
contents.   The hash is calculated once and used for both measurement
and appraisal.  If the file integrity appraisal fails (eg. hash
comparison or signature failure), IMA prevents the kexec files from
being used.

> and IMA will avoid future modification based on the hash calculated?
> If this is the purpose I think it should be wrong because kexe file buffers  
> will be freed at the end of kexec_file_load syscall.

The ima_hash_and _process_file() call calculates the file's hash, adds
the hash to the IMA measurement list and appraises the file's integrity.
On integrity failure, in this case, it prevents the kexec files from
being used, causing the buffers to be freed.

Mimi

> > 
> > Changelog v2:
> > - Calculate the file hash from the in memory buffer (suggested by Dave 
> > Young)
> > - Rename ima_read_and_process_file() to ima_hash_and_process_file()
> > - replace individual case statements with range:
> > KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1
> > v1:
> > - Instead of ima_read_and_process_file() allocating memory, the caller
> > allocates and frees the memory.
> > - Moved the kexec measurement/appraisal call to copy_file_from_fd(). The
> > same call now measures and appraises both the kexec image and initramfs.
> > 
> > Signed-off-by: Mimi Zohar 
> > ---
> >  Documentation/ABI/testing/ima_policy  |  2 +-
> >  include/linux/ima.h   | 16 ++
> >  kernel/kexec_file.c   | 24 
> >  security/integrity/iint.c |  1 +
> >  security/integrity/ima/ima.h  | 21 --
> >  security/integrity/ima/ima_api.c  |  6 +++--
> >  security/integrity/ima/ima_appraise.c | 11 --
> >  security/integrity/ima/ima_main.c | 41 
> > ---
> >  security/integrity/ima/ima_policy.c   | 38 
> >  security/integrity/integrity.h|  7 --
> >  10 files changed, 127 insertions(+), 40 deletions(-)
> > 
> > diff --git a/Documentation/ABI/testing/ima_policy 
> > b/Documentation/ABI/testing/ima_policy
> > index 0a378a8..e80f767 100644
> > --- a/Documentation/ABI/testing/ima_policy
> > +++ b/Documentation/ABI/testing/ima_policy
> > @@ -26,7 +26,7 @@ Description:
> > option: [[appraise_type=]] [permit_directio]
> >  
> > base:   func:= 
> > [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
> > -   [FIRMWARE_CHECK]
> > +   [FIRMWARE_CHECK] [KEXEC_CHECK] [INITRAMFS_CHECK]
> > mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
> >[[^]MAY_EXEC]
> > 

Re: [Linux-ima-devel] [PATCH v2 4/7] ima: measure and appraise kexec image and initramfs

[Dave Young]

Hi, Mimi

CCing kexec list, not all kexec people subscribed to IMA list.
I just subscribed to it since Vivek CCed me last time about the V1 of this
series.

On 12/23/15 at 06:55pm, Mimi Zohar wrote:
> This patch defines a new IMA hook ima_hash_and_process_file() for
> measuring and appraising files read by the kernel.  The caller loads
> the file into memory before calling this function, which calculates
> the hash followed by the normal IMA policy based processing.
> 
> Two new IMA policy functions named KEXEC_CHECK and INITRAMFS_CHECK
> are defined for measuring, appraising or auditing the kexec image
> and initramfs.

Could you help us understand why do we need it first.

I think I do not really understand the purpose of the IMA handling
about kexec kernel and initramfs.

* Does the files in disk space have already contains some hash values 
and when kernel load it IMA functions will do some checking? But seems I do not
see such handling..

* Does it try to calculate the hash of the file buffer after copying,
and IMA will avoid future modification based on the hash calculated?
If this is the purpose I think it should be wrong because kexe file buffers  
will be freed at the end of kexec_file_load syscall.

> 
> Changelog v2:
> - Calculate the file hash from the in memory buffer (suggested by Dave Young)
> - Rename ima_read_and_process_file() to ima_hash_and_process_file()
> - replace individual case statements with range:
>   KEXEC_CHECK ... IMA_MAX_READ_CHECK - 1
> v1:
> - Instead of ima_read_and_process_file() allocating memory, the caller
> allocates and frees the memory.
> - Moved the kexec measurement/appraisal call to copy_file_from_fd(). The
> same call now measures and appraises both the kexec image and initramfs.
> 
> Signed-off-by: Mimi Zohar 
> ---
>  Documentation/ABI/testing/ima_policy  |  2 +-
>  include/linux/ima.h   | 16 ++
>  kernel/kexec_file.c   | 24 
>  security/integrity/iint.c |  1 +
>  security/integrity/ima/ima.h  | 21 --
>  security/integrity/ima/ima_api.c  |  6 +++--
>  security/integrity/ima/ima_appraise.c | 11 --
>  security/integrity/ima/ima_main.c | 41 
> ---
>  security/integrity/ima/ima_policy.c   | 38 
>  security/integrity/integrity.h|  7 --
>  10 files changed, 127 insertions(+), 40 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/ima_policy 
> b/Documentation/ABI/testing/ima_policy
> index 0a378a8..e80f767 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -26,7 +26,7 @@ Description:
>   option: [[appraise_type=]] [permit_directio]
>  
>   base:   func:= 
> [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
> - [FIRMWARE_CHECK]
> + [FIRMWARE_CHECK] [KEXEC_CHECK] [INITRAMFS_CHECK]
>   mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
>  [[^]MAY_EXEC]
>   fsmagic:= hex value
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 120ccc5..020de0f 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -13,6 +13,12 @@
>  #include 
>  struct linux_binprm;
>  
> +enum ima_read_hooks {
> + KEXEC_CHECK = 1,
> + INITRAMFS_CHECK,
> + IMA_MAX_READ_CHECK
> +};
> +
>  #ifdef CONFIG_IMA
>  extern int ima_bprm_check(struct linux_binprm *bprm);
>  extern int ima_file_check(struct file *file, int mask, int opened);
> @@ -20,6 +26,9 @@ extern void ima_file_free(struct file *file);
>  extern int ima_file_mmap(struct file *file, unsigned long prot);
>  extern int ima_module_check(struct file *file);
>  extern int ima_fw_from_file(struct file *file, char *buf, size_t size);
> +extern int ima_hash_and_process_file(struct file *file,
> +  void *buf, size_t size,
> +  enum ima_read_hooks read_func);
>  
>  #else
>  static inline int ima_bprm_check(struct linux_binprm *bprm)
> @@ -52,6 +61,13 @@ static inline int ima_fw_from_file(struct file *file, char 
> *buf, size_t size)
>   return 0;
>  }
>  
> +static inline int ima_hash_and_process_file(struct file *file,
> + void *buf, size_t size,
> + enum ima_read_hooks read_func)
> +{
> + return 0;
> +}
> +
>  #endif /* CONFIG_IMA */
>  
>  #ifdef CONFIG_IMA_APPRAISE
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index b70ada0..1d0d998 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -18,6 +18,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -33,7 +34,8 @@ size_t __weak kexec_purgatory_size = 0;
>  
>  static int kexec_calculate_store_digests(struct