Menu
▾
▴
linux-ima-devel
|
From: Matthew G. <mj...@go...> - 2017-09-08 17:49:46
|
It may be desirable to perform appraisal after credentials are
committed, for instance in the case where validation is only required if
the binary has transitioned into a privileged security context. Add an
additional call into IMA in the committed_credentials security hook and
abort execution if it fails.
Signed-off-by: Matthew Garrett <mj...@go...>
---
Documentation/ABI/testing/ima_policy | 2 +-
arch/x86/ia32/ia32_aout.c | 4 +++-
fs/binfmt_aout.c | 4 +++-
fs/binfmt_elf.c | 5 ++++-
fs/binfmt_elf_fdpic.c | 5 ++++-
fs/binfmt_flat.c | 4 +++-
fs/exec.c | 8 ++++++--
include/linux/binfmts.h | 2 +-
include/linux/ima.h | 6 ++++++
include/linux/security.h | 5 +++--
security/integrity/iint.c | 1 +
security/integrity/ima/ima.h | 1 +
security/integrity/ima/ima_api.c | 2 +-
security/integrity/ima/ima_appraise.c | 8 ++++++++
security/integrity/ima/ima_main.c | 24 +++++++++++++++++++++++-
security/integrity/ima/ima_policy.c | 4 ++++
security/integrity/integrity.h | 9 +++++++--
security/security.c | 3 ++-
18 files changed, 81 insertions(+), 16 deletions(-)
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index e76432b9954d..5dc9eed035fb 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -25,7 +25,7 @@ Description:
[obj_user=] [obj_role=] [obj_type=]]
option: [[appraise_type=]] [permit_directio]
- base: func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
+ base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
[FIRMWARE_CHECK]
[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
index 8d0879f1d42c..5eaedc31661b 100644
--- a/arch/x86/ia32/ia32_aout.c
+++ b/arch/x86/ia32/ia32_aout.c
@@ -312,7 +312,9 @@ static int load_aout_binary(struct linux_binprm *bprm)
if (retval < 0)
return retval;
- install_exec_creds(bprm);
+ retval = install_exec_creds(bprm);
+ if (retval)
+ return retval;
if (N_MAGIC(ex) == OMAGIC) {
unsigned long text_addr, map_size;
diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
index 9be82c4e14a4..5eb778710a46 100644
--- a/fs/binfmt_aout.c
+++ b/fs/binfmt_aout.c
@@ -256,7 +256,9 @@ static int load_aout_binary(struct linux_binprm * bprm)
if (retval < 0)
return retval;
- install_exec_creds(bprm);
+ retval = install_exec_creds(bprm);
+ if (retval)
+ return retval;
if (N_MAGIC(ex) == OMAGIC) {
unsigned long text_addr, map_size;
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 6466153f2bf0..0f0463e8bcb8 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -865,7 +865,10 @@ static int load_elf_binary(struct linux_binprm *bprm)
current->flags |= PF_RANDOMIZE;
setup_new_exec(bprm);
- install_exec_creds(bprm);
+
+ retval = install_exec_creds(bprm);
+ if (retval)
+ goto out_free_dentry;
/* Do this so that we can load the interpreter, if need be. We will
change some of these later */
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index cf93a4fad012..066f81d31d7b 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -432,7 +432,10 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
current->mm->start_stack = current->mm->start_brk + stack_size;
#endif
- install_exec_creds(bprm);
+ retval = install_exec_creds(bprm);
+ if (retval)
+ goto error;
+
if (create_elf_fdpic_tables(bprm, current->mm,
&exec_params, &interp_params) < 0)
goto error;
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index a1e6860b6f46..61cc1099d8a6 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -958,7 +958,9 @@ static int load_flat_binary(struct linux_binprm *bprm)
}
}
- install_exec_creds(bprm);
+ retval = install_exec_creds(bprm);
+ if (retval)
+ return retval;
set_binfmt(&flat_format);
diff --git a/fs/exec.c b/fs/exec.c
index 62175cbcc801..8923f0ce5d57 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1418,8 +1418,10 @@ EXPORT_SYMBOL(bprm_change_interp);
/*
* install the new credentials for this executable
*/
-void install_exec_creds(struct linux_binprm *bprm)
+int install_exec_creds(struct linux_binprm *bprm)
{
+ int ret = 0;
+
security_bprm_committing_creds(bprm);
commit_creds(bprm->cred);
@@ -1438,8 +1440,10 @@ void install_exec_creds(struct linux_binprm *bprm)
* ptrace_attach() from altering our determination of the task's
* credentials; any time after this it may be unlocked.
*/
- security_bprm_committed_creds(bprm);
+ ret = security_bprm_committed_creds(bprm);
mutex_unlock(¤t->signal->cred_guard_mutex);
+
+ return ret;
}
EXPORT_SYMBOL(install_exec_creds);
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 3ae9013eeaaa..4d60d2c432d9 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -121,7 +121,7 @@ extern int bprm_change_interp(char *interp, struct linux_binprm *bprm);
extern int copy_strings_kernel(int argc, const char *const *argv,
struct linux_binprm *bprm);
extern int prepare_bprm_creds(struct linux_binprm *bprm);
-extern void install_exec_creds(struct linux_binprm *bprm);
+extern int install_exec_creds(struct linux_binprm *bprm);
extern void set_binfmt(struct linux_binfmt *new);
extern ssize_t read_code(struct file *, unsigned long, loff_t, size_t);
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0e4647e0eb60..f9a64f94b0d3 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -16,6 +16,7 @@ struct linux_binprm;
#ifdef CONFIG_IMA
extern int ima_bprm_check(struct linux_binprm *bprm);
+extern int ima_creds_check(struct linux_binprm *bprm);
extern int ima_file_check(struct file *file, int mask, int opened);
extern void ima_file_free(struct file *file);
extern int ima_file_mmap(struct file *file, unsigned long prot);
@@ -34,6 +35,11 @@ static inline int ima_bprm_check(struct linux_binprm *bprm)
return 0;
}
+static inline int ima_creds_check(struct linux_binprm *bprm)
+{
+ return 0;
+}
+
static inline int ima_file_check(struct file *file, int mask, int opened)
{
return 0;
diff --git a/include/linux/security.h b/include/linux/security.h
index b6ea1dc9cc9d..21763448dc89 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -231,7 +231,7 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages);
int security_bprm_set_creds(struct linux_binprm *bprm);
int security_bprm_check(struct linux_binprm *bprm);
void security_bprm_committing_creds(struct linux_binprm *bprm);
-void security_bprm_committed_creds(struct linux_binprm *bprm);
+int security_bprm_committed_creds(struct linux_binprm *bprm);
int security_bprm_secureexec(struct linux_binprm *bprm);
int security_sb_alloc(struct super_block *sb);
void security_sb_free(struct super_block *sb);
@@ -537,8 +537,9 @@ static inline void security_bprm_committing_creds(struct linux_binprm *bprm)
{
}
-static inline void security_bprm_committed_creds(struct linux_binprm *bprm)
+static inline int security_bprm_committed_creds(struct linux_binprm *bprm)
{
+ return 0;
}
static inline int security_bprm_secureexec(struct linux_binprm *bprm)
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 6fc888ca468e..ad30094a58b4 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -78,6 +78,7 @@ static void iint_free(struct integrity_iint_cache *iint)
iint->ima_mmap_status = INTEGRITY_UNKNOWN;
iint->ima_bprm_status = INTEGRITY_UNKNOWN;
iint->ima_read_status = INTEGRITY_UNKNOWN;
+ iint->ima_creds_status = INTEGRITY_UNKNOWN;
iint->evm_status = INTEGRITY_UNKNOWN;
iint->measured_pcrs = 0;
kmem_cache_free(iint_cache, iint);
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d52b487ad259..547ea832bb1b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -177,6 +177,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
hook(FILE_CHECK) \
hook(MMAP_CHECK) \
hook(BPRM_CHECK) \
+ hook(CREDS_CHECK) \
hook(POST_SETATTR) \
hook(MODULE_CHECK) \
hook(FIRMWARE_CHECK) \
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index c2edba8de35e..0c19bb423570 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -165,7 +165,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
* The policy is defined in terms of keypairs:
* subj=, obj=, type=, func=, mask=, fsmagic=
* subj,obj, and type: are LSM specific.
- * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
+ * func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
* mask: contains the permission mask
* fsmagic: hex value
*
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 809ba70fbbbf..edb82e722a0d 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -86,6 +86,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
return iint->ima_mmap_status;
case BPRM_CHECK:
return iint->ima_bprm_status;
+ case CREDS_CHECK:
+ return iint->ima_creds_status;
case FILE_CHECK:
case POST_SETATTR:
return iint->ima_file_status;
@@ -106,6 +108,9 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint,
case BPRM_CHECK:
iint->ima_bprm_status = status;
break;
+ case CREDS_CHECK:
+ iint->ima_creds_status = status;
+ break;
case FILE_CHECK:
case POST_SETATTR:
iint->ima_file_status = status;
@@ -127,6 +132,9 @@ static void ima_cache_flags(struct integrity_iint_cache *iint,
case BPRM_CHECK:
iint->flags |= (IMA_BPRM_APPRAISED | IMA_APPRAISED);
break;
+ case CREDS_CHECK:
+ iint->flags |= (IMA_CREDS_APPRAISED | IMA_APPRAISED);
+ break;
case FILE_CHECK:
case POST_SETATTR:
iint->flags |= (IMA_FILE_APPRAISED | IMA_APPRAISED);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 2aebb7984437..5be8307a1dd1 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -14,7 +14,7 @@
*
* File: ima_main.c
* implements the IMA hooks: ima_bprm_check, ima_file_mmap,
- * and ima_file_check.
+ * ima_creds_check and ima_file_check.
*/
#include <linux/module.h>
#include <linux/file.h>
@@ -306,6 +306,28 @@ int ima_bprm_check(struct linux_binprm *bprm)
BPRM_CHECK, 0);
}
+/**
+ * ima_creds_check - based on policy, collect/store measurement.
+ * @bprm: contains the linux_binprm structure
+ *
+ * The OS protects against an executable file, already open for write,
+ * from being executed in deny_write_access() and an executable file,
+ * already open for execute, from being modified in get_write_access().
+ * So we can be certain that what we verify and measure here is actually
+ * what is being executed.
+ *
+ * This is identical to ima_bprm_check, except called after child credentials
+ * have been committed.
+ *
+ * On success return 0. On integrity appraisal error, assuming the file
+ * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
+ */
+int ima_creds_check(struct linux_binprm *bprm)
+{
+ return process_measurement(bprm->file, NULL, 0, MAY_EXEC,
+ CREDS_CHECK, 0);
+}
+
/**
* ima_path_check - based on policy, collect/store measurement.
* @file: pointer to the file to be measured
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 95209a5f8595..a6e14c532627 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -339,6 +339,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
return IMA_MMAP_APPRAISE;
case BPRM_CHECK:
return IMA_BPRM_APPRAISE;
+ case CREDS_CHECK:
+ return IMA_CREDS_APPRAISE;
case FILE_CHECK:
case POST_SETATTR:
return IMA_FILE_APPRAISE;
@@ -691,6 +693,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
entry->func = MMAP_CHECK;
else if (strcmp(args[0].from, "BPRM_CHECK") == 0)
entry->func = BPRM_CHECK;
+ else if (strcmp(args[0].from, "CREDS_CHECK") == 0)
+ entry->func = CREDS_CHECK;
else if (strcmp(args[0].from, "KEXEC_KERNEL_CHECK") ==
0)
entry->func = KEXEC_KERNEL_CHECK;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index a53e7e4ab06c..45ba0e4501d6 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -48,10 +48,14 @@
#define IMA_BPRM_APPRAISED 0x00002000
#define IMA_READ_APPRAISE 0x00004000
#define IMA_READ_APPRAISED 0x00008000
+#define IMA_CREDS_APPRAISE 0x00010000
+#define IMA_CREDS_APPRAISED 0x00020000
#define IMA_APPRAISE_SUBMASK (IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \
- IMA_BPRM_APPRAISE | IMA_READ_APPRAISE)
+ IMA_BPRM_APPRAISE | IMA_READ_APPRAISE | \
+ IMA_CREDS_APPRAISE)
#define IMA_APPRAISED_SUBMASK (IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \
- IMA_BPRM_APPRAISED | IMA_READ_APPRAISED)
+ IMA_BPRM_APPRAISED | IMA_READ_APPRAISED | \
+ IMA_CREDS_APPRAISED)
enum evm_ima_xattr_type {
IMA_XATTR_DIGEST = 0x01,
@@ -108,6 +112,7 @@ struct integrity_iint_cache {
enum integrity_status ima_mmap_status:4;
enum integrity_status ima_bprm_status:4;
enum integrity_status ima_read_status:4;
+ enum integrity_status ima_creds_status:4;
enum integrity_status evm_status:4;
struct ima_digest_data *ima_hash;
};
diff --git a/security/security.c b/security/security.c
index 30132378d103..bdb5cd5c8859 100644
--- a/security/security.c
+++ b/security/security.c
@@ -346,9 +346,10 @@ void security_bprm_committing_creds(struct linux_binprm *bprm)
call_void_hook(bprm_committing_creds, bprm);
}
-void security_bprm_committed_creds(struct linux_binprm *bprm)
+int security_bprm_committed_creds(struct linux_binprm *bprm)
{
call_void_hook(bprm_committed_creds, bprm);
+ return ima_creds_check(bprm);
}
int security_bprm_secureexec(struct linux_binprm *bprm)
--
2.14.1.581.gf28d330327-goog
|
|
From: Matthew G. <mj...@go...> - 2017-10-09 17:45:54
|
On Fri, Sep 8, 2017 at 10:43 AM, Matthew Garrett <mj...@go...> wrote: > It may be desirable to perform appraisal after credentials are > committed, for instance in the case where validation is only required if > the binary has transitioned into a privileged security context. Add an > additional call into IMA in the committed_credentials security hook and > abort execution if it fails. Hi, Any feedback on this? |
|
From: Mimi Z. <zo...@li...> - 2017-10-10 22:22:08
|
On Fri, 2017-09-08 at 10:43 -0700, Matthew Garrett wrote:
> It may be desirable to perform appraisal after credentials are
> committed, for instance in the case where validation is only required if
> the binary has transitioned into a privileged security context. Add an
> additional call into IMA in the committed_credentials security hook and
> abort execution if it fails.
>
> Signed-off-by: Matthew Garrett <mj...@go...>
I was hoping we could replace the existing bprm_check with the new
creds_check, but not all of the binfmt's registered are covered. Only
those that call install_exec_creds() are covered. This should
probably be reflected in the ima_creds_check() description.
Otherwise, the patch looks good.
Mimi
> ---
> Documentation/ABI/testing/ima_policy | 2 +-
> arch/x86/ia32/ia32_aout.c | 4 +++-
> fs/binfmt_aout.c | 4 +++-
> fs/binfmt_elf.c | 5 ++++-
> fs/binfmt_elf_fdpic.c | 5 ++++-
> fs/binfmt_flat.c | 4 +++-
> fs/exec.c | 8 ++++++--
> include/linux/binfmts.h | 2 +-
> include/linux/ima.h | 6 ++++++
> include/linux/security.h | 5 +++--
> security/integrity/iint.c | 1 +
> security/integrity/ima/ima.h | 1 +
> security/integrity/ima/ima_api.c | 2 +-
> security/integrity/ima/ima_appraise.c | 8 ++++++++
> security/integrity/ima/ima_main.c | 24 +++++++++++++++++++++++-
> security/integrity/ima/ima_policy.c | 4 ++++
> security/integrity/integrity.h | 9 +++++++--
> security/security.c | 3 ++-
> 18 files changed, 81 insertions(+), 16 deletions(-)
>
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index e76432b9954d..5dc9eed035fb 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -25,7 +25,7 @@ Description:
> [obj_user=] [obj_role=] [obj_type=]]
> option: [[appraise_type=]] [permit_directio]
>
> - base: func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
> + base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
> [FIRMWARE_CHECK]
> [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
> mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
> diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
> index 8d0879f1d42c..5eaedc31661b 100644
> --- a/arch/x86/ia32/ia32_aout.c
> +++ b/arch/x86/ia32/ia32_aout.c
> @@ -312,7 +312,9 @@ static int load_aout_binary(struct linux_binprm *bprm)
> if (retval < 0)
> return retval;
>
> - install_exec_creds(bprm);
> + retval = install_exec_creds(bprm);
> + if (retval)
> + return retval;
>
> if (N_MAGIC(ex) == OMAGIC) {
> unsigned long text_addr, map_size;
> diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
> index 9be82c4e14a4..5eb778710a46 100644
> --- a/fs/binfmt_aout.c
> +++ b/fs/binfmt_aout.c
> @@ -256,7 +256,9 @@ static int load_aout_binary(struct linux_binprm * bprm)
> if (retval < 0)
> return retval;
>
> - install_exec_creds(bprm);
> + retval = install_exec_creds(bprm);
> + if (retval)
> + return retval;
>
> if (N_MAGIC(ex) == OMAGIC) {
> unsigned long text_addr, map_size;
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 6466153f2bf0..0f0463e8bcb8 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -865,7 +865,10 @@ static int load_elf_binary(struct linux_binprm *bprm)
> current->flags |= PF_RANDOMIZE;
>
> setup_new_exec(bprm);
> - install_exec_creds(bprm);
> +
> + retval = install_exec_creds(bprm);
> + if (retval)
> + goto out_free_dentry;
>
> /* Do this so that we can load the interpreter, if need be. We will
> change some of these later */
> diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
> index cf93a4fad012..066f81d31d7b 100644
> --- a/fs/binfmt_elf_fdpic.c
> +++ b/fs/binfmt_elf_fdpic.c
> @@ -432,7 +432,10 @@ static int load_elf_fdpic_binary(struct linux_binprm *bprm)
> current->mm->start_stack = current->mm->start_brk + stack_size;
> #endif
>
> - install_exec_creds(bprm);
> + retval = install_exec_creds(bprm);
> + if (retval)
> + goto error;
> +
> if (create_elf_fdpic_tables(bprm, current->mm,
> &exec_params, &interp_params) < 0)
> goto error;
> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index a1e6860b6f46..61cc1099d8a6 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -958,7 +958,9 @@ static int load_flat_binary(struct linux_binprm *bprm)
> }
> }
>
> - install_exec_creds(bprm);
> + retval = install_exec_creds(bprm);
> + if (retval)
> + return retval;
>
> set_binfmt(&flat_format);
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 62175cbcc801..8923f0ce5d57 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1418,8 +1418,10 @@ EXPORT_SYMBOL(bprm_change_interp);
> /*
> * install the new credentials for this executable
> */
> -void install_exec_creds(struct linux_binprm *bprm)
> +int install_exec_creds(struct linux_binprm *bprm)
> {
> + int ret = 0;
> +
> security_bprm_committing_creds(bprm);
>
> commit_creds(bprm->cred);
> @@ -1438,8 +1440,10 @@ void install_exec_creds(struct linux_binprm *bprm)
> * ptrace_attach() from altering our determination of the task's
> * credentials; any time after this it may be unlocked.
> */
> - security_bprm_committed_creds(bprm);
> + ret = security_bprm_committed_creds(bprm);
> mutex_unlock(¤t->signal->cred_guard_mutex);
> +
> + return ret;
> }
> EXPORT_SYMBOL(install_exec_creds);
>
> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> index 3ae9013eeaaa..4d60d2c432d9 100644
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -121,7 +121,7 @@ extern int bprm_change_interp(char *interp, struct linux_binprm *bprm);
> extern int copy_strings_kernel(int argc, const char *const *argv,
> struct linux_binprm *bprm);
> extern int prepare_bprm_creds(struct linux_binprm *bprm);
> -extern void install_exec_creds(struct linux_binprm *bprm);
> +extern int install_exec_creds(struct linux_binprm *bprm);
> extern void set_binfmt(struct linux_binfmt *new);
> extern ssize_t read_code(struct file *, unsigned long, loff_t, size_t);
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 0e4647e0eb60..f9a64f94b0d3 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -16,6 +16,7 @@ struct linux_binprm;
>
> #ifdef CONFIG_IMA
> extern int ima_bprm_check(struct linux_binprm *bprm);
> +extern int ima_creds_check(struct linux_binprm *bprm);
> extern int ima_file_check(struct file *file, int mask, int opened);
> extern void ima_file_free(struct file *file);
> extern int ima_file_mmap(struct file *file, unsigned long prot);
> @@ -34,6 +35,11 @@ static inline int ima_bprm_check(struct linux_binprm *bprm)
> return 0;
> }
>
> +static inline int ima_creds_check(struct linux_binprm *bprm)
> +{
> + return 0;
> +}
> +
> static inline int ima_file_check(struct file *file, int mask, int opened)
> {
> return 0;
> diff --git a/include/linux/security.h b/include/linux/security.h
> index b6ea1dc9cc9d..21763448dc89 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -231,7 +231,7 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages);
> int security_bprm_set_creds(struct linux_binprm *bprm);
> int security_bprm_check(struct linux_binprm *bprm);
> void security_bprm_committing_creds(struct linux_binprm *bprm);
> -void security_bprm_committed_creds(struct linux_binprm *bprm);
> +int security_bprm_committed_creds(struct linux_binprm *bprm);
> int security_bprm_secureexec(struct linux_binprm *bprm);
> int security_sb_alloc(struct super_block *sb);
> void security_sb_free(struct super_block *sb);
> @@ -537,8 +537,9 @@ static inline void security_bprm_committing_creds(struct linux_binprm *bprm)
> {
> }
>
> -static inline void security_bprm_committed_creds(struct linux_binprm *bprm)
> +static inline int security_bprm_committed_creds(struct linux_binprm *bprm)
> {
> + return 0;
> }
>
> static inline int security_bprm_secureexec(struct linux_binprm *bprm)
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index 6fc888ca468e..ad30094a58b4 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -78,6 +78,7 @@ static void iint_free(struct integrity_iint_cache *iint)
> iint->ima_mmap_status = INTEGRITY_UNKNOWN;
> iint->ima_bprm_status = INTEGRITY_UNKNOWN;
> iint->ima_read_status = INTEGRITY_UNKNOWN;
> + iint->ima_creds_status = INTEGRITY_UNKNOWN;
> iint->evm_status = INTEGRITY_UNKNOWN;
> iint->measured_pcrs = 0;
> kmem_cache_free(iint_cache, iint);
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index d52b487ad259..547ea832bb1b 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -177,6 +177,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
> hook(FILE_CHECK) \
> hook(MMAP_CHECK) \
> hook(BPRM_CHECK) \
> + hook(CREDS_CHECK) \
> hook(POST_SETATTR) \
> hook(MODULE_CHECK) \
> hook(FIRMWARE_CHECK) \
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c2edba8de35e..0c19bb423570 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -165,7 +165,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
> * The policy is defined in terms of keypairs:
> * subj=, obj=, type=, func=, mask=, fsmagic=
> * subj,obj, and type: are LSM specific.
> - * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK
> + * func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK
> * mask: contains the permission mask
> * fsmagic: hex value
> *
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 809ba70fbbbf..edb82e722a0d 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -86,6 +86,8 @@ enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
> return iint->ima_mmap_status;
> case BPRM_CHECK:
> return iint->ima_bprm_status;
> + case CREDS_CHECK:
> + return iint->ima_creds_status;
> case FILE_CHECK:
> case POST_SETATTR:
> return iint->ima_file_status;
> @@ -106,6 +108,9 @@ static void ima_set_cache_status(struct integrity_iint_cache *iint,
> case BPRM_CHECK:
> iint->ima_bprm_status = status;
> break;
> + case CREDS_CHECK:
> + iint->ima_creds_status = status;
> + break;
> case FILE_CHECK:
> case POST_SETATTR:
> iint->ima_file_status = status;
> @@ -127,6 +132,9 @@ static void ima_cache_flags(struct integrity_iint_cache *iint,
> case BPRM_CHECK:
> iint->flags |= (IMA_BPRM_APPRAISED | IMA_APPRAISED);
> break;
> + case CREDS_CHECK:
> + iint->flags |= (IMA_CREDS_APPRAISED | IMA_APPRAISED);
> + break;
> case FILE_CHECK:
> case POST_SETATTR:
> iint->flags |= (IMA_FILE_APPRAISED | IMA_APPRAISED);
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 2aebb7984437..5be8307a1dd1 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -14,7 +14,7 @@
> *
> * File: ima_main.c
> * implements the IMA hooks: ima_bprm_check, ima_file_mmap,
> - * and ima_file_check.
> + * ima_creds_check and ima_file_check.
> */
> #include <linux/module.h>
> #include <linux/file.h>
> @@ -306,6 +306,28 @@ int ima_bprm_check(struct linux_binprm *bprm)
> BPRM_CHECK, 0);
> }
>
> +/**
> + * ima_creds_check - based on policy, collect/store measurement.
> + * @bprm: contains the linux_binprm structure
> + *
> + * The OS protects against an executable file, already open for write,
> + * from being executed in deny_write_access() and an executable file,
> + * already open for execute, from being modified in get_write_access().
> + * So we can be certain that what we verify and measure here is actually
> + * what is being executed.
> + *
> + * This is identical to ima_bprm_check, except called after child credentials
> + * have been committed.
> + *
> + * On success return 0. On integrity appraisal error, assuming the file
> + * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
> + */
> +int ima_creds_check(struct linux_binprm *bprm)
> +{
> + return process_measurement(bprm->file, NULL, 0, MAY_EXEC,
> + CREDS_CHECK, 0);
> +}
> +
> /**
> * ima_path_check - based on policy, collect/store measurement.
> * @file: pointer to the file to be measured
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 95209a5f8595..a6e14c532627 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -339,6 +339,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
> return IMA_MMAP_APPRAISE;
> case BPRM_CHECK:
> return IMA_BPRM_APPRAISE;
> + case CREDS_CHECK:
> + return IMA_CREDS_APPRAISE;
> case FILE_CHECK:
> case POST_SETATTR:
> return IMA_FILE_APPRAISE;
> @@ -691,6 +693,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
> entry->func = MMAP_CHECK;
> else if (strcmp(args[0].from, "BPRM_CHECK") == 0)
> entry->func = BPRM_CHECK;
> + else if (strcmp(args[0].from, "CREDS_CHECK") == 0)
> + entry->func = CREDS_CHECK;
> else if (strcmp(args[0].from, "KEXEC_KERNEL_CHECK") ==
> 0)
> entry->func = KEXEC_KERNEL_CHECK;
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index a53e7e4ab06c..45ba0e4501d6 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -48,10 +48,14 @@
> #define IMA_BPRM_APPRAISED 0x00002000
> #define IMA_READ_APPRAISE 0x00004000
> #define IMA_READ_APPRAISED 0x00008000
> +#define IMA_CREDS_APPRAISE 0x00010000
> +#define IMA_CREDS_APPRAISED 0x00020000
> #define IMA_APPRAISE_SUBMASK (IMA_FILE_APPRAISE | IMA_MMAP_APPRAISE | \
> - IMA_BPRM_APPRAISE | IMA_READ_APPRAISE)
> + IMA_BPRM_APPRAISE | IMA_READ_APPRAISE | \
> + IMA_CREDS_APPRAISE)
> #define IMA_APPRAISED_SUBMASK (IMA_FILE_APPRAISED | IMA_MMAP_APPRAISED | \
> - IMA_BPRM_APPRAISED | IMA_READ_APPRAISED)
> + IMA_BPRM_APPRAISED | IMA_READ_APPRAISED | \
> + IMA_CREDS_APPRAISED)
>
> enum evm_ima_xattr_type {
> IMA_XATTR_DIGEST = 0x01,
> @@ -108,6 +112,7 @@ struct integrity_iint_cache {
> enum integrity_status ima_mmap_status:4;
> enum integrity_status ima_bprm_status:4;
> enum integrity_status ima_read_status:4;
> + enum integrity_status ima_creds_status:4;
> enum integrity_status evm_status:4;
> struct ima_digest_data *ima_hash;
> };
> diff --git a/security/security.c b/security/security.c
> index 30132378d103..bdb5cd5c8859 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -346,9 +346,10 @@ void security_bprm_committing_creds(struct linux_binprm *bprm)
> call_void_hook(bprm_committing_creds, bprm);
> }
>
> -void security_bprm_committed_creds(struct linux_binprm *bprm)
> +int security_bprm_committed_creds(struct linux_binprm *bprm)
> {
> call_void_hook(bprm_committed_creds, bprm);
> + return ima_creds_check(bprm);
> }
>
> int security_bprm_secureexec(struct linux_binprm *bprm)
|
|
From: Matthew G. <mj...@go...> - 2017-10-10 22:24:54
|
On Tue, Oct 10, 2017 at 3:21 PM, Mimi Zohar <zo...@li...> wrote: > I was hoping we could replace the existing bprm_check with the new > creds_check, but not all of the binfmt's registered are covered. Only > those that call install_exec_creds() are covered. This should > probably be reflected in the ima_creds_check() description. > Otherwise, the patch looks good. The semantics are different - bprm_check will check sub_user and co against the pre-exec() credentials, creds_check against the post-exec() credentials. That feels like something that could break existing policies, so I think we need to keep them independent. I'll rewrite the description and resend. |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X