From 3ec30113264a7bcd389f51d1738e42da0f41bb5a Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Mon, 8 Jan 2018 13:36:19 -0800 Subject: security: Add a cred_getsecid hook For IMA purposes, we want to be able to obtain the prepared secid in the bprm structure before the credentials are committed. Add a cred_getsecid hook that makes this possible. Signed-off-by: Matthew Garrett Acked-by: Paul Moore Cc: Paul Moore Cc: Stephen Smalley Cc: Casey Schaufler Signed-off-by: Mimi Zohar --- security/security.c | 7 +++++++ security/selinux/hooks.c | 6 ++++++ security/smack/smack_lsm.c | 18 ++++++++++++++++++ 3 files changed, 31 insertions(+) (limited to 'security') diff --git a/security/security.c b/security/security.c index 14c2919..957e8be 100644 --- a/security/security.c +++ b/security/security.c @@ -1005,6 +1005,13 @@ void security_transfer_creds(struct cred *new, const struct cred *old) call_void_hook(cred_transfer, new, old); } +void security_cred_getsecid(const struct cred *c, u32 *secid) +{ + *secid = 0; + call_void_hook(cred_getsecid, c, secid); +} +EXPORT_SYMBOL(security_cred_getsecid); + int security_kernel_act_as(struct cred *new, u32 secid) { return call_int_hook(kernel_act_as, 0, new, secid); diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 8abd542..b7d4473 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3844,6 +3844,11 @@ static void selinux_cred_transfer(struct cred *new, const struct cred *old) *tsec = *old_tsec; } +static void selinux_cred_getsecid(const struct cred *c, u32 *secid) +{ + *secid = cred_sid(c); +} + /* * set the security data for a kernel service * - all the creation contexts are set to unlabelled @@ -6482,6 +6487,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(cred_free, selinux_cred_free), LSM_HOOK_INIT(cred_prepare, selinux_cred_prepare), LSM_HOOK_INIT(cred_transfer, selinux_cred_transfer), + LSM_HOOK_INIT(cred_getsecid, selinux_cred_getsecid), LSM_HOOK_INIT(kernel_act_as, selinux_kernel_act_as), LSM_HOOK_INIT(kernel_create_files_as, selinux_kernel_create_files_as), LSM_HOOK_INIT(kernel_module_request, selinux_kernel_module_request), diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index feada26..ed20d36 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -2050,6 +2050,23 @@ static void smack_cred_transfer(struct cred *new, const struct cred *old) } /** + * smack_cred_getsecid - get the secid corresponding to a creds structure + * @c: the object creds + * @secid: where to put the result + * + * Sets the secid to contain a u32 version of the smack label. + */ +static void smack_cred_getsecid(const struct cred *c, u32 *secid) +{ + struct smack_known *skp; + + rcu_read_lock(); + skp = smk_of_task(c->security); + *secid = skp->smk_secid; + rcu_read_unlock(); +} + +/** * smack_kernel_act_as - Set the subjective context in a set of credentials * @new: points to the set of credentials to be modified. * @secid: specifies the security ID to be set @@ -4733,6 +4750,7 @@ static struct security_hook_list smack_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(cred_free, smack_cred_free), LSM_HOOK_INIT(cred_prepare, smack_cred_prepare), LSM_HOOK_INIT(cred_transfer, smack_cred_transfer), + LSM_HOOK_INIT(cred_getsecid, smack_cred_getsecid), LSM_HOOK_INIT(kernel_act_as, smack_kernel_act_as), LSM_HOOK_INIT(kernel_create_files_as, smack_kernel_create_files_as), LSM_HOOK_INIT(task_setpgid, smack_task_setpgid), -- cgit v1.1 From d906c10d8a31654cb9167c9a2ebc7d3e43820bad Mon Sep 17 00:00:00 2001 From: Matthew Garrett Date: Mon, 8 Jan 2018 13:36:20 -0800 Subject: IMA: Support using new creds in appraisal policy The existing BPRM_CHECK functionality in IMA validates against the credentials of the existing process, not any new credentials that the child process may transition to. Add an additional CREDS_CHECK target and refactor IMA to pass the appropriate creds structure. In ima_bprm_check(), check with both the existing process credentials and the credentials that will be committed when the new process is started. This will not change behaviour unless the system policy is extended to include CREDS_CHECK targets - BPRM_CHECK will continue to check the same credentials that it did previously. After this patch, an IMA policy rule along the lines of: measure func=CREDS_CHECK subj_type=unconfined_t will trigger if a process is executed and runs as unconfined_t, ignoring the context of the parent process. This is in contrast to: measure func=BPRM_CHECK subj_type=unconfined_t which will trigger if the process that calls exec() is already executing in unconfined_t, ignoring the context that the child process executes into. Signed-off-by: Matthew Garrett Signed-off-by: Mimi Zohar Changelog: - initialize ima_creds_status --- security/integrity/iint.c | 2 ++ security/integrity/ima/ima.h | 9 ++++---- security/integrity/ima/ima_api.c | 9 +++++--- security/integrity/ima/ima_appraise.c | 14 ++++++++++-- security/integrity/ima/ima_main.c | 42 ++++++++++++++++++++++++++--------- security/integrity/ima/ima_policy.c | 23 ++++++++++++------- security/integrity/integrity.h | 9 ++++++-- 7 files changed, 79 insertions(+), 29 deletions(-) (limited to 'security') diff --git a/security/integrity/iint.c b/security/integrity/iint.c index 9700e96..f266e4b 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -79,6 +79,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); @@ -158,6 +159,7 @@ static void init_once(void *foo) 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; mutex_init(&iint->mutex); } diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index d52b487..35fe91a 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) \ @@ -191,8 +192,8 @@ enum ima_hooks { }; /* LIM API function definitions */ -int ima_get_action(struct inode *inode, int mask, - enum ima_hooks func, int *pcr); +int ima_get_action(struct inode *inode, const struct cred *cred, u32 secid, + 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, struct file *file, void *buf, loff_t size, @@ -212,8 +213,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 inode *inode, const struct cred *cred, u32 secid, + 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 08fe405..33b4458 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -158,6 +158,8 @@ err_out: /** * ima_get_action - appraise & measure decision based on policy. * @inode: pointer to inode to measure + * @cred: pointer to credentials structure to validate + * @secid: secid of the task being validated * @mask: contains the permission mask (MAY_READ, MAY_WRITE, MAY_EXEC, * MAY_APPEND) * @func: caller identifier @@ -166,20 +168,21 @@ err_out: * 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 * * 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 inode *inode, const struct cred *cred, u32 secid, + int mask, enum ima_hooks func, int *pcr) { int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH; flags &= ima_policy_flag; - return ima_match_policy(inode, func, mask, flags, pcr); + return ima_match_policy(inode, cred, secid, func, mask, flags, pcr); } /* diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index f2803a4..1b17746 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -50,11 +50,14 @@ bool is_ima_appraise_enabled(void) */ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func) { + u32 secid; + if (!ima_appraise) return 0; - return ima_match_policy(inode, func, mask, IMA_APPRAISE | IMA_HASH, - NULL); + security_task_getsecid(current, &secid); + return ima_match_policy(inode, current_cred(), secid, func, mask, + IMA_APPRAISE | IMA_HASH, NULL); } static int ima_fix_xattr(struct dentry *dentry, @@ -87,6 +90,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; @@ -107,6 +112,8 @@ 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; case FILE_CHECK: case POST_SETATTR: iint->ima_file_status = status; @@ -128,6 +135,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 2cfb0c7..a5d225f 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -167,8 +167,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 file *file, const struct cred *cred, + u32 secid, 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; @@ -190,7 +191,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(inode, cred, secid, mask, func, &pcr); violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) && (ima_policy_flag & IMA_MEASURE)); if (!action && !violation_check) @@ -324,9 +325,14 @@ out: */ int ima_file_mmap(struct file *file, unsigned long prot) { - if (file && (prot & PROT_EXEC)) - return process_measurement(file, NULL, 0, MAY_EXEC, - MMAP_CHECK, 0); + u32 secid; + + if (file && (prot & PROT_EXEC)) { + security_task_getsecid(current, &secid); + return process_measurement(file, current_cred(), secid, NULL, + 0, MAY_EXEC, MMAP_CHECK, 0); + } + return 0; } @@ -345,8 +351,18 @@ 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, - BPRM_CHECK, 0); + int ret; + u32 secid; + + security_task_getsecid(current, &secid); + ret = process_measurement(bprm->file, current_cred(), secid, NULL, 0, + MAY_EXEC, BPRM_CHECK, 0); + if (ret) + return ret; + + security_cred_getsecid(bprm->cred, &secid); + return process_measurement(bprm->file, bprm->cred, secid, NULL, 0, + MAY_EXEC, CREDS_CHECK, 0); } /** @@ -361,7 +377,10 @@ 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, + u32 secid; + + security_task_getsecid(current, &secid); + return process_measurement(file, current_cred(), secid, NULL, 0, mask & (MAY_READ | MAY_WRITE | MAY_EXEC | MAY_APPEND), FILE_CHECK, opened); } @@ -440,6 +459,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size, enum kernel_read_file_id read_id) { enum ima_hooks func; + u32 secid; if (!file && read_id == READING_FIRMWARE) { if ((ima_appraise & IMA_APPRAISE_FIRMWARE) && @@ -462,7 +482,9 @@ 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); + security_task_getsecid(current, &secid); + return process_measurement(file, current_cred(), secid, 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 915f557..e3da29a 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -243,16 +243,17 @@ static void ima_lsm_update_rules(void) * ima_match_rules - determine whether an inode matches the measure rule. * @rule: a pointer to a rule * @inode: a pointer to an inode + * @cred: a pointer to a credentials structure for user validation + * @secid: the secid of the task to be validated * @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, + const struct cred *cred, u32 secid, enum ima_hooks func, int mask) { - struct task_struct *tsk = current; - const struct cred *cred = current_cred(); int i; if ((rule->flags & IMA_FUNC) && @@ -287,7 +288,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, return false; for (i = 0; i < MAX_LSM_RULES; i++) { int rc = 0; - u32 osid, sid; + u32 osid; int retried = 0; if (!rule->lsm[i].rule) @@ -307,8 +308,7 @@ retry: case LSM_SUBJ_USER: case LSM_SUBJ_ROLE: case LSM_SUBJ_TYPE: - security_task_getsecid(tsk, &sid); - rc = security_filter_rule_match(sid, + rc = security_filter_rule_match(secid, rule->lsm[i].type, Audit_equal, rule->lsm[i].rule, @@ -341,6 +341,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; @@ -353,6 +355,9 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func) /** * ima_match_policy - decision based on LSM and other conditions * @inode: pointer to an inode for which the policy decision is being made + * @cred: pointer to a credentials structure for which the policy decision is + * being made + * @secid: LSM secid of the task to be validated * @func: IMA hook identifier * @mask: requested action (MAY_READ | MAY_WRITE | MAY_APPEND | MAY_EXEC) * @pcr: set the pcr to extend @@ -364,8 +369,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 inode *inode, const struct cred *cred, u32 secid, + enum ima_hooks func, int mask, int flags, int *pcr) { struct ima_rule_entry *entry; int action = 0, actmask = flags | (flags << 1); @@ -376,7 +381,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, inode, cred, secid, func, mask)) continue; action |= entry->flags & IMA_ACTION_FLAGS; @@ -713,6 +718,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 50a8e33..843ae23 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -51,10 +51,14 @@ #define IMA_BPRM_APPRAISED 0x00020000 #define IMA_READ_APPRAISE 0x00040000 #define IMA_READ_APPRAISED 0x00080000 +#define IMA_CREDS_APPRAISE 0x00100000 +#define IMA_CREDS_APPRAISED 0x00200000 #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) /* iint cache atomic_flags */ #define IMA_CHANGE_XATTR 0 @@ -121,6 +125,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; }; -- cgit v1.1 From 57b56ac6fecb05c3192586e4892572dd13d972de Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Wed, 21 Feb 2018 11:33:37 -0500 Subject: ima: fail file signature verification on non-init mounted filesystems FUSE can be mounted by unprivileged users either today with fusermount installed with setuid, or soon with the upcoming patches to allow FUSE mounts in a non-init user namespace. This patch addresses the new unprivileged non-init mounted filesystems, which are untrusted, by failing the signature verification. This patch defines two new flags SB_I_IMA_UNVERIFIABLE_SIGNATURE and SB_I_UNTRUSTED_MOUNTER. Signed-off-by: Mimi Zohar Cc: Miklos Szeredi Cc: Seth Forshee Cc: Dongsu Park Cc: Alban Crequy Acked-by: Serge Hallyn Acked-by: "Eric W. Biederman" --- security/integrity/ima/ima_appraise.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 1b17746..4bafb39 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -302,7 +302,19 @@ int ima_appraise_measurement(enum ima_hooks func, } out: - if (status != INTEGRITY_PASS) { + /* + * File signatures on some filesystems can not be properly verified. + * On these filesytems, that are mounted by an untrusted mounter, + * fail the file signature verification. + */ + if ((inode->i_sb->s_iflags & + (SB_I_IMA_UNVERIFIABLE_SIGNATURE | SB_I_UNTRUSTED_MOUNTER)) == + (SB_I_IMA_UNVERIFIABLE_SIGNATURE | SB_I_UNTRUSTED_MOUNTER)) { + status = INTEGRITY_FAIL; + cause = "unverifiable-signature"; + integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename, + op, cause, rc, 0); + } else if (status != INTEGRITY_PASS) { if ((ima_appraise & IMA_APPRAISE_FIX) && (!xattr_value || xattr_value->type != EVM_IMA_XATTR_DIGSIG)) { @@ -319,6 +331,7 @@ out: } else { ima_cache_flags(iint, func); } + ima_set_cache_status(iint, func, status); return status; } -- cgit v1.1 From d77ccdc644a59b412d8e101576134c90a0aa6797 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Wed, 21 Feb 2018 11:35:20 -0500 Subject: ima: re-evaluate files on privileged mounted filesystems This patch addresses the fuse privileged mounted filesystems in a "secure" environment, with a correctly enforced security policy, which is willing to assume the inherent risk of specific fuse filesystems that are well defined and properly implemented. As there is no way for the kernel to detect file changes, the kernel ignores the cached file integrity results and re-measures, re-appraises, and re-audits the file. Signed-off-by: Mimi Zohar Cc: Miklos Szeredi Cc: Seth Forshee Cc: Dongsu Park Cc: Alban Crequy Acked-by: Serge Hallyn Acked-by: "Eric W. Biederman" --- security/integrity/ima/ima_main.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index a5d225f..f550f25 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -25,6 +25,7 @@ #include #include #include +#include #include "ima.h" @@ -230,9 +231,17 @@ static int process_measurement(struct file *file, const struct cred *cred, IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK | IMA_ACTION_FLAGS); - if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags)) - /* reset all flags if ima_inode_setxattr was called */ + /* + * Re-evaulate the file if either the xattr has changed or the + * kernel has no way of detecting file change on the filesystem. + * (Limited to privileged mounted filesystems.) + */ + if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags) || + ((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) && + !(inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER))) { iint->flags &= ~IMA_DONE_MASK; + iint->measured_pcrs = 0; + } /* Determine if already appraised/measured based on bitmask * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED, -- cgit v1.1 From a9a4935d44b58c858a81393694bc232a96cdcbd4 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Sat, 10 Mar 2018 23:07:34 -0500 Subject: ima: clear IMA_HASH The IMA_APPRAISE and IMA_HASH policies overlap. Clear IMA_HASH properly. Fixes: da1b0029f527 ("ima: support new "hash" and "dont_hash" policy actions") Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_policy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index e3da29a..40557c0 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -389,7 +389,7 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid, action |= entry->action & IMA_DO_MASK; if (entry->action & IMA_APPRAISE) { action |= get_subaction(entry, func); - action ^= IMA_HASH; + action &= ~IMA_HASH; } if (entry->action & IMA_DO_MASK) -- cgit v1.1 From 9e67028e76514a8ee279d7d006dfb8069b5115ab Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Wed, 21 Feb 2018 11:36:32 -0500 Subject: ima: fail signature verification based on policy This patch addresses the fuse privileged mounted filesystems in environments which are unwilling to accept the risk of trusting the signature verification and want to always fail safe, but are for example using a pre-built kernel. This patch defines a new builtin policy named "fail_securely", which can be specified on the boot command line as an argument to "ima_policy=". Signed-off-by: Mimi Zohar Cc: Miklos Szeredi Cc: Seth Forshee Cc: Dongsu Park Cc: Alban Crequy Acked-by: Serge Hallyn Acked-by: "Eric W. Biederman" --- security/integrity/ima/ima_appraise.c | 11 ++++++----- security/integrity/ima/ima_main.c | 3 ++- security/integrity/ima/ima_policy.c | 5 +++++ security/integrity/integrity.h | 1 + 4 files changed, 14 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 4bafb39..0c5f94b 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -304,12 +304,13 @@ int ima_appraise_measurement(enum ima_hooks func, out: /* * File signatures on some filesystems can not be properly verified. - * On these filesytems, that are mounted by an untrusted mounter, - * fail the file signature verification. + * When such filesystems are mounted by an untrusted mounter or on a + * system not willing to accept such a risk, fail the file signature + * verification. */ - if ((inode->i_sb->s_iflags & - (SB_I_IMA_UNVERIFIABLE_SIGNATURE | SB_I_UNTRUSTED_MOUNTER)) == - (SB_I_IMA_UNVERIFIABLE_SIGNATURE | SB_I_UNTRUSTED_MOUNTER)) { + if ((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) && + ((inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) || + (iint->flags & IMA_FAIL_UNVERIFIABLE_SIGS))) { status = INTEGRITY_FAIL; cause = "unverifiable-signature"; integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename, diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index f550f25..5d122da 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -238,7 +238,8 @@ static int process_measurement(struct file *file, const struct cred *cred, */ if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags) || ((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) && - !(inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER))) { + !(inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) && + !(action & IMA_FAIL_UNVERIFIABLE_SIGS))) { iint->flags &= ~IMA_DONE_MASK; iint->measured_pcrs = 0; } diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 40557c0..51a4cd9 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -188,6 +188,7 @@ __setup("ima_tcb", default_measure_policy_setup); static bool ima_use_appraise_tcb __initdata; static bool ima_use_secure_boot __initdata; +static bool ima_fail_unverifiable_sigs __ro_after_init; static int __init policy_setup(char *str) { char *p; @@ -201,6 +202,8 @@ static int __init policy_setup(char *str) ima_use_appraise_tcb = true; else if (strcmp(p, "secure_boot") == 0) ima_use_secure_boot = true; + else if (strcmp(p, "fail_securely") == 0) + ima_fail_unverifiable_sigs = true; } return 1; @@ -390,6 +393,8 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid, if (entry->action & IMA_APPRAISE) { action |= get_subaction(entry, func); action &= ~IMA_HASH; + if (ima_fail_unverifiable_sigs) + action |= IMA_FAIL_UNVERIFIABLE_SIGS; } if (entry->action & IMA_DO_MASK) diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 843ae23..8224880 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -35,6 +35,7 @@ #define IMA_PERMIT_DIRECTIO 0x02000000 #define IMA_NEW_FILE 0x04000000 #define EVM_IMMUTABLE_DIGSIG 0x08000000 +#define IMA_FAIL_UNVERIFIABLE_SIGS 0x10000000 #define IMA_DO_MASK (IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \ IMA_HASH | IMA_APPRAISE_SUBMASK) -- cgit v1.1 From 1a82cee3e69d8dc89dc2e9995770e7cab0a4c9d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hern=C3=A1n=20Gonzalez?= Date: Tue, 27 Feb 2018 19:16:59 -0300 Subject: evm: Move evm_hmac and evm_hash from evm_main.c to evm_crypto.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These variables are not used where they are was defined. There is no point in declaring them there as extern. Move and constify them, saving 2 bytes. Function old new delta init_desc 273 271 -2 Total: Before=2112094, After=2112092, chg -0.00% Signed-off-by: Hernán Gonzalez Tested-by: Fengguang Wu Signed-off-by: Mimi Zohar --- security/integrity/evm/evm.h | 2 -- security/integrity/evm/evm_crypto.c | 3 +++ security/integrity/evm/evm_main.c | 2 -- 3 files changed, 3 insertions(+), 4 deletions(-) (limited to 'security') diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h index 0482539..45c4a89 100644 --- a/security/integrity/evm/evm.h +++ b/security/integrity/evm/evm.h @@ -31,8 +31,6 @@ EVM_ALLOW_METADATA_WRITES) extern int evm_initialized; -extern char *evm_hmac; -extern char *evm_hash; #define EVM_ATTR_FSUUID 0x0001 diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 691f3e0..a46fba3 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -37,6 +37,9 @@ static DEFINE_MUTEX(mutex); static unsigned long evm_set_key_flags; +static char * const evm_hmac = "hmac(sha1)"; +static char * const evm_hash = "sha1"; + /** * evm_set_key() - set EVM HMAC key from the kernel * @key: pointer to a buffer with the key data diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index a8d5028..826926d 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -33,8 +33,6 @@ int evm_initialized; static char *integrity_status_msg[] = { "pass", "pass_immutable", "fail", "no_label", "no_xattrs", "unknown" }; -char *evm_hmac = "hmac(sha1)"; -char *evm_hash = "sha1"; int evm_hmac_attrs; char *evm_config_xattrnames[] = { -- cgit v1.1 From 17d7b0af062f587f9cbedce3cb6f39a784651e2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hern=C3=A1n=20Gonzalez?= Date: Tue, 27 Feb 2018 19:17:00 -0300 Subject: evm: Constify *integrity_status_msg[] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There is no gain from doing this except for some self-documenting. Signed-off-by: Hernán Gonzalez Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security') diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 826926d..7a968fa 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -30,7 +30,7 @@ int evm_initialized; -static char *integrity_status_msg[] = { +static const char * const integrity_status_msg[] = { "pass", "pass_immutable", "fail", "no_label", "no_xattrs", "unknown" }; int evm_hmac_attrs; -- cgit v1.1 From fac37c628fd5d68fd7298d9b57ae8601ee1b4723 Mon Sep 17 00:00:00 2001 From: Jiandi An Date: Tue, 6 Mar 2018 23:26:26 -0600 Subject: ima: Fix Kconfig to select TPM 2.0 CRB interface TPM_CRB driver provides TPM CRB 2.0 support. If it is built as a module, the TPM chip is registered after IMA init. tpm_pcr_read() in IMA fails and displays the following message even though eventually there is a TPM chip on the system. ima: No TPM chip found, activating TPM-bypass! (rc=-19) Fix IMA Kconfig to select TPM_CRB so TPM_CRB driver is built in the kernel and initializes before IMA. Signed-off-by: Jiandi An Signed-off-by: Mimi Zohar --- security/integrity/ima/Kconfig | 1 + 1 file changed, 1 insertion(+) (limited to 'security') diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 35ef693..6a8f677 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -10,6 +10,7 @@ config IMA select CRYPTO_HASH_INFO select TCG_TPM if HAS_IOMEM && !UML select TCG_TIS if TCG_TPM && X86 + select TCG_CRB if TCG_TPM && ACPI select TCG_IBMVTPM if TCG_TPM && PPC_PSERIES help The Trusted Computing Group(TCG) runtime Integrity -- cgit v1.1 From e456ef88ae8c9ffd303e970c28dcce4474c3d356 Mon Sep 17 00:00:00 2001 From: Tycho Andersen Date: Thu, 8 Mar 2018 16:08:36 -0700 Subject: ima: drop vla in ima_audit_measurement() In keeping with the directive to get rid of VLAs [1], let's drop the VLA from ima_audit_measurement(). We need to adjust the return type of ima_audit_measurement, because now this function can fail if an allocation fails. [1]: https://lkml.org/lkml/2018/3/7/621 v2: just use audit_log_format instead of doing a second allocation v3: ignore failures in ima_audit_measurement() Signed-off-by: Tycho Andersen Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_api.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 33b4458..bf88236 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -311,14 +311,17 @@ void ima_audit_measurement(struct integrity_iint_cache *iint, const unsigned char *filename) { struct audit_buffer *ab; - char hash[(iint->ima_hash->length * 2) + 1]; + char *hash; const char *algo_name = hash_algo_name[iint->ima_hash->algo]; - char algo_hash[sizeof(hash) + strlen(algo_name) + 2]; int i; if (iint->flags & IMA_AUDITED) return; + hash = kzalloc((iint->ima_hash->length * 2) + 1, GFP_KERNEL); + if (!hash) + return; + for (i = 0; i < iint->ima_hash->length; i++) hex_byte_pack(hash + (i * 2), iint->ima_hash->digest[i]); hash[i * 2] = '\0'; @@ -326,18 +329,19 @@ void ima_audit_measurement(struct integrity_iint_cache *iint, ab = audit_log_start(current->audit_context, GFP_KERNEL, AUDIT_INTEGRITY_RULE); if (!ab) - return; + goto out; audit_log_format(ab, "file="); audit_log_untrustedstring(ab, filename); - audit_log_format(ab, " hash="); - snprintf(algo_hash, sizeof(algo_hash), "%s:%s", algo_name, hash); - audit_log_untrustedstring(ab, algo_hash); + audit_log_format(ab, " hash=\"%s:%s\"", algo_name, hash); audit_log_task_info(ab, current); audit_log_end(ab); iint->flags |= IMA_AUDITED; +out: + kfree(hash); + return; } /* -- cgit v1.1 From 11c60f23ed1367298207efb3bbf985203a95f480 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Wed, 14 Mar 2018 17:20:17 -0300 Subject: integrity: Remove unused macro IMA_ACTION_RULE_FLAGS This macro isn't used anymore since commit 0d73a55208e9 ("ima: re-introduce own integrity cache lock"), so remove it. Signed-off-by: Thiago Jung Bauermann Acked-by: Serge Hallyn Signed-off-by: Mimi Zohar --- security/integrity/integrity.h | 1 - 1 file changed, 1 deletion(-) (limited to 'security') diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 8224880..5e58e02 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -30,7 +30,6 @@ /* iint cache flags */ #define IMA_ACTION_FLAGS 0xff000000 -#define IMA_ACTION_RULE_FLAGS 0x06000000 #define IMA_DIGSIG_REQUIRED 0x01000000 #define IMA_PERMIT_DIRECTIO 0x02000000 #define IMA_NEW_FILE 0x04000000 -- cgit v1.1 From 1775cb87b063cd60a5021c38412f6024f93cc376 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Wed, 14 Mar 2018 17:20:18 -0300 Subject: ima: Simplify ima_eventsig_init() The "goto out" statement doesn't have any purpose since there's no cleanup to be done when returning early, so remove it. This also makes the rc variable unnecessary so remove it as well. Also, the xattr_len and fmt variables are redundant so remove them as well. Signed-off-by: Thiago Jung Bauermann Acked-by: Serge Hallyn Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_template_lib.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c index 28af43f..5afaa53d 100644 --- a/security/integrity/ima/ima_template_lib.c +++ b/security/integrity/ima/ima_template_lib.c @@ -378,16 +378,11 @@ int ima_eventname_ng_init(struct ima_event_data *event_data, int ima_eventsig_init(struct ima_event_data *event_data, struct ima_field_data *field_data) { - enum data_formats fmt = DATA_FMT_HEX; struct evm_ima_xattr_data *xattr_value = event_data->xattr_value; - int xattr_len = event_data->xattr_len; - int rc = 0; if ((!xattr_value) || (xattr_value->type != EVM_IMA_XATTR_DIGSIG)) - goto out; + return 0; - rc = ima_write_template_field_data(xattr_value, xattr_len, fmt, - field_data); -out: - return rc; + return ima_write_template_field_data(xattr_value, event_data->xattr_len, + DATA_FMT_HEX, field_data); } -- cgit v1.1 From f5e51fa368fd6612608bc36f39e55cde08ce0039 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Thu, 15 Mar 2018 17:33:42 -0300 Subject: ima: Improvements in ima_appraise_measurement() Replace nested ifs in the EVM xattr verification logic with a switch statement, making the code easier to understand. Also, add comments to the if statements in the out section and constify the cause variable. Signed-off-by: Mimi Zohar Signed-off-by: Thiago Jung Bauermann Acked-by: Serge Hallyn --- security/integrity/ima/ima_appraise.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) (limited to 'security') diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 0c5f94b..8bd7a07 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -215,7 +215,7 @@ int ima_appraise_measurement(enum ima_hooks func, int xattr_len, int opened) { static const char op[] = "appraise_data"; - char *cause = "unknown"; + const char *cause = "unknown"; struct dentry *dentry = file_dentry(file); struct inode *inode = d_backing_inode(dentry); enum integrity_status status = INTEGRITY_UNKNOWN; @@ -241,16 +241,22 @@ int ima_appraise_measurement(enum ima_hooks func, } status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint); - if ((status != INTEGRITY_PASS) && - (status != INTEGRITY_PASS_IMMUTABLE) && - (status != INTEGRITY_UNKNOWN)) { - if ((status == INTEGRITY_NOLABEL) - || (status == INTEGRITY_NOXATTRS)) - cause = "missing-HMAC"; - else if (status == INTEGRITY_FAIL) - cause = "invalid-HMAC"; + switch (status) { + case INTEGRITY_PASS: + case INTEGRITY_PASS_IMMUTABLE: + case INTEGRITY_UNKNOWN: + break; + case INTEGRITY_NOXATTRS: /* No EVM protected xattrs. */ + case INTEGRITY_NOLABEL: /* No security.evm xattr. */ + cause = "missing-HMAC"; + goto out; + case INTEGRITY_FAIL: /* Invalid HMAC/signature. */ + cause = "invalid-HMAC"; goto out; + default: + WARN_ONCE(true, "Unexpected integrity status %d\n", status); } + switch (xattr_value->type) { case IMA_XATTR_DIGEST_NG: /* first byte contains algorithm id */ @@ -316,17 +322,20 @@ out: integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename, op, cause, rc, 0); } else if (status != INTEGRITY_PASS) { + /* Fix mode, but don't replace file signatures. */ if ((ima_appraise & IMA_APPRAISE_FIX) && (!xattr_value || xattr_value->type != EVM_IMA_XATTR_DIGSIG)) { if (!ima_fix_xattr(dentry, iint)) status = INTEGRITY_PASS; - } else if ((inode->i_size == 0) && - (iint->flags & IMA_NEW_FILE) && - (xattr_value && - xattr_value->type == EVM_IMA_XATTR_DIGSIG)) { + } + + /* Permit new files with file signatures, but without data. */ + if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE && + xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG) { status = INTEGRITY_PASS; } + integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename, op, cause, rc, 0); } else { -- cgit v1.1 From 70946c4ac2a9e036e3cf7048cd670507f5074b04 Mon Sep 17 00:00:00 2001 From: Sascha Hauer Date: Thu, 1 Mar 2018 13:38:45 +0100 Subject: evm: check for remount ro in progress before writing EVM might update the evm xattr while the VFS performs a remount to readonly mode. This is not properly checked for, additionally check the s_readonly_remount superblock flag before writing. The bug can for example be observed with UBIFS. UBIFS checks the free space on the device before and after a remount. With EVM enabled the free space sometimes differs between both checks. Signed-off-by: Sascha Hauer Signed-off-by: Mimi Zohar --- security/integrity/evm/evm_main.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'security') diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 7a968fa..9ea9c19 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -124,6 +124,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, struct evm_ima_xattr_data *xattr_data = NULL; struct evm_ima_xattr_data calc; enum integrity_status evm_status = INTEGRITY_PASS; + struct inode *inode; int rc, xattr_len; if (iint && (iint->evm_status == INTEGRITY_PASS || @@ -178,12 +179,15 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry, (const char *)xattr_data, xattr_len, calc.digest, sizeof(calc.digest)); if (!rc) { + inode = d_backing_inode(dentry); + if (xattr_data->type == EVM_XATTR_PORTABLE_DIGSIG) { if (iint) iint->flags |= EVM_IMMUTABLE_DIGSIG; evm_status = INTEGRITY_PASS_IMMUTABLE; - } else if (!IS_RDONLY(d_backing_inode(dentry)) && - !IS_IMMUTABLE(d_backing_inode(dentry))) { + } else if (!IS_RDONLY(inode) && + !(inode->i_sb->s_readonly_remount) && + !IS_IMMUTABLE(inode)) { evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len); -- cgit v1.1 From 1c070b18d448211644cc0050e6c9a4a204bbcf63 Mon Sep 17 00:00:00 2001 From: Martin Townsend Date: Thu, 22 Mar 2018 13:46:01 +0000 Subject: ima: Add smackfs to the default appraise/measure list This is required to use SMACK and IMA/EVM together. Add it to the default nomeasure/noappraise list like other pseudo filesystems. Signed-off-by: Martin Townsend Acked-by: Casey Schaufler Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_policy.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'security') diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 51a4cd9..d89bebf 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -96,6 +96,7 @@ static struct ima_rule_entry dont_measure_rules[] __ro_after_init = { {.action = DONT_MEASURE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_MEASURE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_MEASURE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = SMACK_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_MEASURE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_MEASURE, .fsmagic = CGROUP2_SUPER_MAGIC, @@ -141,6 +142,7 @@ static struct ima_rule_entry default_appraise_rules[] __ro_after_init = { {.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = SMACK_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_APPRAISE, .fsmagic = NSFS_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_APPRAISE, .fsmagic = CGROUP2_SUPER_MAGIC, .flags = IMA_FSMAGIC}, -- cgit v1.1 From ab60368ab6a452466885ef4edf0cefd089465132 Mon Sep 17 00:00:00 2001 From: Petr Vorel Date: Fri, 23 Mar 2018 14:41:08 +0100 Subject: ima: Fallback to the builtin hash algorithm IMA requires having it's hash algorithm be compiled-in due to it's early use. The default IMA algorithm is protected by Kconfig to be compiled-in. The ima_hash kernel parameter allows to choose the hash algorithm. When the specified algorithm is not available or available as a module, IMA initialization fails, which leads to a kernel panic (mknodat syscall calls ima_post_path_mknod()). Therefore as fallback we force IMA to use the default builtin Kconfig hash algorithm. Fixed crash: $ grep CONFIG_CRYPTO_MD4 .config CONFIG_CRYPTO_MD4=m [ 0.000000] Command line: BOOT_IMAGE=/boot/vmlinuz-4.12.14-2.3-default root=UUID=74ae8202-9ca7-4e39-813b-22287ec52f7a video=1024x768-16 plymouth.ignore-serial-consoles console=ttyS0 console=tty resume=/dev/disk/by-path/pci-0000:00:07.0-part3 splash=silent showopts ima_hash=md4 ... [ 1.545190] ima: Can not allocate md4 (reason: -2) ... [ 2.610120] BUG: unable to handle kernel NULL pointer dereference at (null) [ 2.611903] IP: ima_match_policy+0x23/0x390 [ 2.612967] PGD 0 P4D 0 [ 2.613080] Oops: 0000 [#1] SMP [ 2.613080] Modules linked in: autofs4 [ 2.613080] Supported: Yes [ 2.613080] CPU: 0 PID: 1 Comm: systemd Not tainted 4.12.14-2.3-default #1 [ 2.613080] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.0.0-prebuilt.qemu-project.org 04/01/2014 [ 2.613080] task: ffff88003e2d0040 task.stack: ffffc90000190000 [ 2.613080] RIP: 0010:ima_match_policy+0x23/0x390 [ 2.613080] RSP: 0018:ffffc90000193e88 EFLAGS: 00010296 [ 2.613080] RAX: 0000000000000000 RBX: 000000000000000c RCX: 0000000000000004 [ 2.613080] RDX: 0000000000000010 RSI: 0000000000000001 RDI: ffff880037071728 [ 2.613080] RBP: 0000000000008000 R08: 0000000000000000 R09: 0000000000000000 [ 2.613080] R10: 0000000000000008 R11: 61c8864680b583eb R12: 00005580ff10086f [ 2.613080] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000008000 [ 2.613080] FS: 00007f5c1da08940(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000 [ 2.613080] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 2.613080] CR2: 0000000000000000 CR3: 0000000037002000 CR4: 00000000003406f0 [ 2.613080] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 2.613080] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 2.613080] Call Trace: [ 2.613080] ? shmem_mknod+0xbf/0xd0 [ 2.613080] ima_post_path_mknod+0x1c/0x40 [ 2.613080] SyS_mknod+0x210/0x220 [ 2.613080] entry_SYSCALL_64_fastpath+0x1a/0xa5 [ 2.613080] RIP: 0033:0x7f5c1bfde570 [ 2.613080] RSP: 002b:00007ffde1c90dc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000085 [ 2.613080] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f5c1bfde570 [ 2.613080] RDX: 0000000000000000 RSI: 0000000000008000 RDI: 00005580ff10086f [ 2.613080] RBP: 00007ffde1c91040 R08: 00005580ff10086f R09: 0000000000000000 [ 2.613080] R10: 0000000000104000 R11: 0000000000000246 R12: 00005580ffb99660 [ 2.613080] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000002 [ 2.613080] Code: 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 41 57 41 56 44 8d 14 09 41 55 41 54 55 53 44 89 d3 09 cb 48 83 ec 38 48 8b 05 c5 03 29 01 <4c> 8b 20 4c 39 e0 0f 84 d7 01 00 00 4c 89 44 24 08 89 54 24 20 [ 2.613080] RIP: ima_match_policy+0x23/0x390 RSP: ffffc90000193e88 [ 2.613080] CR2: 0000000000000000 [ 2.613080] ---[ end trace 9a9f0a8a73079f6a ]--- [ 2.673052] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009 [ 2.673052] [ 2.675337] Kernel Offset: disabled [ 2.676405] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000009 Signed-off-by: Petr Vorel Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_crypto.c | 2 ++ security/integrity/ima/ima_main.c | 13 +++++++++++++ 2 files changed, 15 insertions(+) (limited to 'security') diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 205bc69..4e085a1 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -73,6 +73,8 @@ int __init ima_init_crypto(void) hash_algo_name[ima_hash_algo], rc); return rc; } + pr_info("Allocated hash algorithm: %s\n", + hash_algo_name[ima_hash_algo]); return 0; } diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 5d122da..74d0bd7 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -16,6 +16,9 @@ * implements the IMA hooks: ima_bprm_check, ima_file_mmap, * and ima_file_check. */ + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + #include #include #include @@ -504,6 +507,16 @@ static int __init init_ima(void) ima_init_template_list(); hash_setup(CONFIG_IMA_DEFAULT_HASH); error = ima_init(); + + if (error && strcmp(hash_algo_name[ima_hash_algo], + CONFIG_IMA_DEFAULT_HASH) != 0) { + pr_info("Allocating %s failed, going to use default hash algorithm %s\n", + hash_algo_name[ima_hash_algo], CONFIG_IMA_DEFAULT_HASH); + hash_setup_done = 0; + hash_setup(CONFIG_IMA_DEFAULT_HASH); + error = ima_init(); + } + if (!error) { ima_initialized = 1; ima_update_policy_flag(); -- cgit v1.1