|
|
| [Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] |
On Mon, 2012-06-04 at 11:30 -0700, Peter Moody wrote:
> This adds a new policy rule 'measure_and_audit' to IMA which audit
> logs the pid, ppid, uid, euid, auid, path and hash of a measurement.
> I've not added a Kconfig for this, but if that's necessary, I
> certainly can.
>
> This is quite a bit simpler than my previous attempt with GIH (thanks, Mimi).
>
>
> Signed-off-by: Peter Moody <pmoody@xxxxxxxxxx>
The IMA-appraisal patch set, available from
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity
#next-ima-appraisal, extends the policy language with "appraise" and
"dont_appraise". I can imagine someone enabling appraisal, but not
measurement, yet still wanting to audit measurements.
Instead of using the keyword 'Opt_measure_audit', how about using
'Opt_audit'? Then instead of calling ima_audit_measurement() from
ima_store_measurement(), like I originally suggested, you could call it
from process_measurements(). Something like the following,
if (action & IMA_MEASURE)
ima_store_measurement(iint, file, filename);
if (action & IMA_APPRAISE)
rc = ima_appraise_measurement(iint, file, filename);
if (action & IMA_AUDIT)
ima_audit_measurement()
Additional comments inline.
> ---
> security/integrity/ima/ima.h | 4 ++-
> security/integrity/ima/ima_api.c | 47 ++++++++++++++++++++++++++++++++---
> security/integrity/ima/ima_main.c | 10 ++++++-
> security/integrity/ima/ima_policy.c | 13 +++++++--
> 4 files changed, 64 insertions(+), 10 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 3ccf7ac..62fc655 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -28,6 +28,7 @@
>
> enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_ASCII };
> enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
> +enum ima_action { UNKNOWN = -1, DONT_MEASURE = 0, MEASURE, MEASURE_AND_AUDIT };
>
> /* digest size for IMA, fits SHA1 or MD5 */
> #define IMA_DIGEST_SIZE SHA1_DIGEST_SIZE
> @@ -102,10 +103,11 @@ int ima_must_measure(struct inode *inode, int
> mask, int function);
> int ima_collect_measurement(struct integrity_iint_cache *iint,
> struct file *file);
> void ima_store_measurement(struct integrity_iint_cache *iint, struct
> file *file,
> - const unsigned char *filename);
> + const unsigned char *filename, int audit);
> int ima_store_template(struct ima_template_entry *entry, int violation,
> struct inode *inode);
> void ima_template_show(struct seq_file *m, void *e, enum ima_show_type show);
> +void ima_audit_measurement(struct ima_template_data *entry, struct file *file);
>
> /* rbtree tree calls to lookup, insert, delete
> * integrity data associated with an inode.
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index 88a2788..93e70aa 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -105,15 +105,16 @@ err_out:
> * mask: contains the permission mask
> * fsmagic: hex value
> *
> - * Return 0 to measure. For matching a DONT_MEASURE policy, no policy,
> - * or other error, return an error code.
> + * Return the action code if the action is measure or measure_and_audit.
> + * For matching a DONT_MEASURE policy, no policy, or other error,
> + * return an error code.
> */
> int ima_must_measure(struct inode *inode, int mask, int function)
> {
> int must_measure;
>
> must_measure = ima_match_policy(inode, function, mask);
> - return must_measure ? 0 : -EACCES;
> + return (must_measure > 0) ? must_measure : -EACCES;
> }
>
> /*
> @@ -158,7 +159,8 @@ int ima_collect_measurement(struct
> integrity_iint_cache *iint,
> * Must be called with iint->mutex held.
> */
> void ima_store_measurement(struct integrity_iint_cache *iint,
> - struct file *file, const unsigned char *filename)
> + struct file *file, const unsigned char *filename,
> + int audit)
> {
> const char *op = "add_template_measure";
> const char *audit_cause = "ENOMEM";
> @@ -180,6 +182,43 @@ void ima_store_measurement(struct
> integrity_iint_cache *iint,
> result = ima_store_template(entry, violation, inode);
> if (!result || result == -EEXIST)
> iint->flags |= IMA_MEASURED;
> +
> + if (audit)
> + ima_audit_measurement(&entry->template, file);
> +
> if (result < 0)
> kfree(entry);
> }
> +
> +void ima_audit_measurement(struct ima_template_data *entry, struct file *file)
> +{
> + struct audit_buffer *ab;
> + char *hash;
> + int i;
> +
> + hash = (char*)kmalloc(PAGE_SIZE, GFP_TEMPORARY);
PAGE_SIZE?
> + if (!hash)
> + goto out;
Instead of dropping the audit msg, perhaps log the existing info.
> + memset(hash, 0, PAGE_SIZE);
kzalloc allocates and zeroes out the buffer, but I'm not convinced you
need to zero out the entire buffer. How about null terminating the
string?
> + for (i = 0; i < SHA1_DIGEST_SIZE; i++)
> + snprintf(hash + (i * 2), 4, "%02x", entry->digest[i]);
> +
> + ab = audit_log_start(current->audit_context, GFP_KERNEL,
> + AUDIT_INTEGRITY_RULE);
> + if (!ab)
> + goto out;
> +
> + audit_log_format(ab, "pid=%d ppid=%d uid=%d euid=%d auid=%d ",
> + current->pid, current->real_parent->pid,
> + current->real_cred->uid, current->cred->uid,
> + current->loginuid);
There are a couple of audit routines that you could use here (eg.
audit_get_loginuid(), audit_get_sessionid()). Also you should use
current_cred() instead of current->cred.
> + audit_log_d_path(ab, "path=", &file->f_path);
> + audit_log_format(ab, " hash=%s", hash);
> + audit_log_task_context(ab);
> + audit_log_end(ab);
> +
> +out:
> + if (hash)
> + kfree(hash);
> +}
> diff --git a/security/integrity/ima/ima_main.c
> b/security/integrity/ima/ima_main.c
> index b17be79..38481c0 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -124,13 +124,18 @@ static int process_measurement(struct file
> *file, const unsigned char *filename,
> struct inode *inode = file->f_dentry->d_inode;
> struct integrity_iint_cache *iint;
> int rc = 0;
> + int audit = 0;
>
> if (!ima_initialized || !S_ISREG(inode->i_mode))
> return 0;
>
> rc = ima_must_measure(inode, mask, function);
> - if (rc != 0)
> + if (rc <= 0)
> return rc;
> +
> + if (rc == MEASURE_AND_AUDIT)
> + audit = 1;
> +
> retry:
> iint = integrity_iint_find(inode);
> if (!iint) {
> @@ -148,7 +153,8 @@ retry:
>
> rc = ima_collect_measurement(iint, file);
> if (!rc)
> - ima_store_measurement(iint, file, filename);
> + ima_store_measurement(iint, file, filename, audit);
> +
> out:
> mutex_unlock(&iint->mutex);
> return rc;
> diff --git a/security/integrity/ima/ima_policy.c
> b/security/integrity/ima/ima_policy.c
> index d8edff2..8dca858 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -25,8 +25,6 @@
> #define IMA_FSMAGIC 0x0004
> #define IMA_UID 0x0008
>
> -enum ima_action { UNKNOWN = -1, DONT_MEASURE = 0, MEASURE };
> -
> #define MAX_LSM_RULES 6
> enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE,
> LSM_SUBJ_USER, LSM_SUBJ_ROLE, LSM_SUBJ_TYPE
> @@ -221,7 +219,7 @@ void ima_update_policy(void)
>
> enum {
> Opt_err = -1,
> - Opt_measure = 1, Opt_dont_measure,
> + Opt_measure = 1, Opt_dont_measure, Opt_measure_and_audit,
> Opt_obj_user, Opt_obj_role, Opt_obj_type,
> Opt_subj_user, Opt_subj_role, Opt_subj_type,
> Opt_func, Opt_mask, Opt_fsmagic, Opt_uid
> @@ -230,6 +228,7 @@ enum {
> static match_table_t policy_tokens = {
> {Opt_measure, "measure"},
> {Opt_dont_measure, "dont_measure"},
> + {Opt_measure_and_audit, "measure_and_audit"},
> {Opt_obj_user, "obj_user=%s"},
> {Opt_obj_role, "obj_role=%s"},
> {Opt_obj_type, "obj_type=%s"},
> @@ -304,6 +303,14 @@ static int ima_parse_rule(char *rule, struct
> ima_measure_rule_entry *entry)
>
> entry->action = DONT_MEASURE;
> break;
> + case Opt_measure_and_audit:
> + ima_log_string(ab, "action", "measure_and_audit");
> +
> + if (entry->action != UNKNOWN)
> + result = -EINVAL;
> +
> + entry->action = MEASURE_AND_AUDIT;
> + break;
> case Opt_func:
> ima_log_string(ab, "func", args[0].from);
As discussed above, how about changing this to Opt_audit?
thanks,
Mimi
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
[Fedora Maintainers] [Fedora Desktop] [Fedora SELinux] [Yosemite News] [Yosemite Photos] [KDE Users] [Fedora Tools]
