Menu
▾
▴
Re: [Linux-ima-devel] [PATCH] IMA: Pass bprm through to rule validation
|
From: Mimi Z. <zo...@li...> - 2017-08-31 14:01:09
|
On Tue, 2017-08-29 at 11:42 -0700, Matthew Garrett wrote:
> Right now IMA BPRM validation is performed after setup_creds() but before
> commit_creds(), which means current->cred still refers to the fork()ing
> task rather than the child. This makes it difficult to implement
> appraisal rules that depend on the security context that the task will
> be transitioned to. Passing through bprm to the policy matching allows
> us to do so.
There are other LSM hooks in fs/exec.c. Instead of you modifying
process_measurements(), could you defer the policy check until after
the commit_creds? We could define the corresponding IMA hook.
Mimi
> Signed-off-by: Matthew Garrett <mj...@go...>
>
> --
>
> Does this seem reasonable? I thought about adding an additional match
> type (BPRM_LATE or something) but this seems like a simpler approach.
> ---
> security/integrity/ima/ima.h | 6 +++---
> security/integrity/ima/ima_api.c | 6 ++++--
> security/integrity/ima/ima_appraise.c | 2 +-
> security/integrity/ima/ima_main.c | 15 ++++++++-------
> security/integrity/ima/ima_policy.c | 11 +++++++----
> 5 files changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index d52b487ad259..695614d85201 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -191,7 +191,7 @@ enum ima_hooks {
> };
>
> /* LIM API function definitions */
> -int ima_get_action(struct inode *inode, int mask,
> +int ima_get_action(struct linux_binprm *bprm, struct inode *inode, int mask,
> enum ima_hooks func, int *pcr);
> int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
> int ima_collect_measurement(struct integrity_iint_cache *iint,
> @@ -212,8 +212,8 @@ void ima_free_template_entry(struct ima_template_entry *entry);
> const char *ima_d_path(const struct path *path, char **pathbuf, char *filename);
>
> /* IMA policy related functions */
> -int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
> - int flags, int *pcr);
> +int ima_match_policy(struct linux_binprm *bprm, struct inode *inode,
> + enum ima_hooks func, int mask, int flags, int *pcr);
> void ima_init_policy(void);
> void ima_update_policy(void);
> void ima_update_policy_flag(void);
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c2edba8de35e..1c1a1ffee398 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -156,6 +156,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
>
> /**
> * ima_get_action - appraise & measure decision based on policy.
> + * @bprm: pointer to the BPRM struct to be validated
> * @inode: pointer to inode to measure
> * @mask: contains the permission mask (MAY_READ, MAY_WRITE, MAY_EXEC,
> * MAY_APPEND)
> @@ -172,13 +173,14 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
> * Returns IMA_MEASURE, IMA_APPRAISE mask.
> *
> */
> -int ima_get_action(struct inode *inode, int mask, enum ima_hooks func, int *pcr)
> +int ima_get_action(struct linux_binprm *bprm, struct inode *inode, int mask,
> + enum ima_hooks func, int *pcr)
> {
> int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE;
>
> flags &= ima_policy_flag;
>
> - return ima_match_policy(inode, func, mask, flags, pcr);
> + return ima_match_policy(bprm, inode, func, mask, flags, pcr);
> }
>
> /*
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index 809ba70fbbbf..bcd956fc4a04 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -53,7 +53,7 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func)
> if (!ima_appraise)
> return 0;
>
> - return ima_match_policy(inode, func, mask, IMA_APPRAISE, NULL);
> + return ima_match_policy(NULL, inode, func, mask, IMA_APPRAISE, NULL);
> }
>
> static int ima_fix_xattr(struct dentry *dentry,
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 2aebb7984437..4b1f2630b736 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -155,8 +155,9 @@ void ima_file_free(struct file *file)
> ima_check_last_writer(iint, inode, file);
> }
>
> -static int process_measurement(struct file *file, char *buf, loff_t size,
> - int mask, enum ima_hooks func, int opened)
> +static int process_measurement(struct linux_binprm *bprm, struct file *file,
> + char *buf, loff_t size, int mask,
> + enum ima_hooks func, int opened)
> {
> struct inode *inode = file_inode(file);
> struct integrity_iint_cache *iint = NULL;
> @@ -178,7 +179,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
> * bitmask based on the appraise/audit/measurement policy.
> * Included is the appraise submask.
> */
> - action = ima_get_action(inode, mask, func, &pcr);
> + action = ima_get_action(bprm, inode, mask, func, &pcr);
> violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) &&
> (ima_policy_flag & IMA_MEASURE));
> if (!action && !violation_check)
> @@ -282,7 +283,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
> int ima_file_mmap(struct file *file, unsigned long prot)
> {
> if (file && (prot & PROT_EXEC))
> - return process_measurement(file, NULL, 0, MAY_EXEC,
> + return process_measurement(NULL, file, NULL, 0, MAY_EXEC,
> MMAP_CHECK, 0);
> return 0;
> }
> @@ -302,7 +303,7 @@ int ima_file_mmap(struct file *file, unsigned long prot)
> */
> int ima_bprm_check(struct linux_binprm *bprm)
> {
> - return process_measurement(bprm->file, NULL, 0, MAY_EXEC,
> + return process_measurement(bprm, bprm->file, NULL, 0, MAY_EXEC,
> BPRM_CHECK, 0);
> }
>
> @@ -318,7 +319,7 @@ int ima_bprm_check(struct linux_binprm *bprm)
> */
> int ima_file_check(struct file *file, int mask, int opened)
> {
> - return process_measurement(file, NULL, 0,
> + return process_measurement(NULL, file, NULL, 0,
> mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
> MAY_APPEND), FILE_CHECK, opened);
> }
> @@ -413,7 +414,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
> }
>
> func = read_idmap[read_id] ?: FILE_CHECK;
> - return process_measurement(file, buf, size, MAY_READ, func, 0);
> + return process_measurement(NULL, file, buf, size, MAY_READ, func, 0);
> }
>
> static int __init init_ima(void)
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 95209a5f8595..93f6af4e3a20 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -240,13 +240,15 @@ static void ima_lsm_update_rules(void)
> /**
> * ima_match_rules - determine whether an inode matches the measure rule.
> * @rule: a pointer to a rule
> + * @bprm: a pointer to a binprm structure
> * @inode: a pointer to an inode
> * @func: LIM hook identifier
> * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
> *
> * Returns true on rule match, false on failure.
> */
> -static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode,
> +static bool ima_match_rules(struct ima_rule_entry *rule,
> + struct linux_binprm *bprm, struct inode *inode,
> enum ima_hooks func, int mask)
> {
> struct task_struct *tsk = current;
> @@ -350,6 +352,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
>
> /**
> * ima_match_policy - decision based on LSM and other conditions
> + * @bprm: pointer to a binprm for which the policy decision is being made
> * @inode: pointer to an inode for which the policy decision is being made
> * @func: IMA hook identifier
> * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC)
> @@ -362,8 +365,8 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
> * list when walking it. Reads are many orders of magnitude more numerous
> * than writes so ima_match_policy() is classical RCU candidate.
> */
> -int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
> - int flags, int *pcr)
> +int ima_match_policy(struct linux_binprm *bprm, struct inode *inode,
> + enum ima_hooks func, int mask, int flags, int *pcr)
> {
> struct ima_rule_entry *entry;
> int action = 0, actmask = flags | (flags << 1);
> @@ -374,7 +377,7 @@ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
> if (!(entry->action & actmask))
> continue;
>
> - if (!ima_match_rules(entry, inode, func, mask))
> + if (!ima_match_rules(entry, bprm, inode, func, mask))
> continue;
>
> action |= entry->flags & IMA_ACTION_FLAGS;
|
