Skip to content

Commit 4f2946a

Browse files
nightmaredmimizohar
authored andcommitted
IMA: introduce a new policy option func=SETXATTR_CHECK
While users can restrict the accepted hash algorithms for the security.ima xattr file signature when appraising said file, users cannot restrict the algorithms that can be set on that attribute: any algorithm built in the kernel is accepted on a write. Define a new value for the ima policy option 'func' that restricts globally the hash algorithms accepted when writing the security.ima xattr. When a policy contains a rule of the form appraise func=SETXATTR_CHECK appraise_algos=sha256,sha384,sha512 only values corresponding to one of these three digest algorithms will be accepted for writing the security.ima xattr. Attempting to write the attribute using another algorithm (or "free-form" data) will be denied with an audit log message. In the absence of such a policy rule, the default is still to only accept hash algorithms built in the kernel (with all the limitations that entails). Signed-off-by: THOBY Simon <Simon.THOBY@viveris.fr> Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
1 parent 583a80a commit 4f2946a

5 files changed

Lines changed: 104 additions & 18 deletions

File tree

Documentation/ABI/testing/ima_policy

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,10 @@ Description:
3030
[appraise_flag=] [appraise_algos=] [keyrings=]
3131
base:
3232
func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
33-
[FIRMWARE_CHECK]
33+
[FIRMWARE_CHECK]
3434
[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
3535
[KEXEC_CMDLINE] [KEY_CHECK] [CRITICAL_DATA]
36+
[SETXATTR_CHECK]
3637
mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
3738
[[^]MAY_EXEC]
3839
fsmagic:= hex value
@@ -138,3 +139,9 @@ Description:
138139
keys added to .builtin_trusted_keys or .ima keyring:
139140

140141
measure func=KEY_CHECK keyrings=.builtin_trusted_keys|.ima
142+
143+
Example of the special SETXATTR_CHECK appraise rule, that
144+
restricts the hash algorithms allowed when writing to the
145+
security.ima xattr of a file:
146+
147+
appraise func=SETXATTR_CHECK appraise_algos=sha256,sha384,sha512

security/integrity/ima/ima.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
4646
/* current content of the policy */
4747
extern int ima_policy_flag;
4848

49+
/* bitset of digests algorithms allowed in the setxattr hook */
50+
extern atomic_t ima_setxattr_allowed_hash_algorithms;
51+
4952
/* set during initialization */
5053
extern int ima_hash_algo __ro_after_init;
5154
extern int ima_sha1_idx __ro_after_init;
@@ -198,6 +201,7 @@ static inline unsigned int ima_hash_key(u8 *digest)
198201
hook(KEXEC_CMDLINE, kexec_cmdline) \
199202
hook(KEY_CHECK, key) \
200203
hook(CRITICAL_DATA, critical_data) \
204+
hook(SETXATTR_CHECK, setxattr_check) \
201205
hook(MAX_CHECK, none)
202206

203207
#define __ima_hook_enumify(ENUM, str) ENUM,
@@ -288,7 +292,7 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
288292
const char *func_data, unsigned int *allowed_algos);
289293
void ima_init_policy(void);
290294
void ima_update_policy(void);
291-
void ima_update_policy_flag(void);
295+
void ima_update_policy_flags(void);
292296
ssize_t ima_parse_add_rule(char *);
293297
void ima_delete_rules(void);
294298
int ima_check_policy(void);

security/integrity/ima/ima_appraise.c

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -595,12 +595,32 @@ static int validate_hash_algo(struct dentry *dentry,
595595
{
596596
char *path = NULL, *pathbuf = NULL;
597597
enum hash_algo xattr_hash_algo;
598+
const char *errmsg = "unavailable-hash-algorithm";
599+
unsigned int allowed_hashes;
598600

599601
xattr_hash_algo = ima_get_hash_algo(xattr_value, xattr_value_len);
600602

601-
if (likely(xattr_hash_algo == ima_hash_algo ||
602-
crypto_has_alg(hash_algo_name[xattr_hash_algo], 0, 0)))
603-
return 0;
603+
allowed_hashes = atomic_read(&ima_setxattr_allowed_hash_algorithms);
604+
605+
if (allowed_hashes) {
606+
/* success if the algorithm is allowed in the ima policy */
607+
if (allowed_hashes & (1U << xattr_hash_algo))
608+
return 0;
609+
610+
/*
611+
* We use a different audit message when the hash algorithm
612+
* is denied by a policy rule, instead of not being built
613+
* in the kernel image
614+
*/
615+
errmsg = "denied-hash-algorithm";
616+
} else {
617+
if (likely(xattr_hash_algo == ima_hash_algo))
618+
return 0;
619+
620+
/* allow any xattr using an algorithm built in the kernel */
621+
if (crypto_has_alg(hash_algo_name[xattr_hash_algo], 0, 0))
622+
return 0;
623+
}
604624

605625
pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
606626
if (!pathbuf)
@@ -609,8 +629,7 @@ static int validate_hash_algo(struct dentry *dentry,
609629
path = dentry_path(dentry, pathbuf, PATH_MAX);
610630

611631
integrity_audit_msg(AUDIT_INTEGRITY_DATA, d_inode(dentry), path,
612-
"set_data", "unavailable-hash-algorithm",
613-
-EACCES, 0);
632+
"set_data", errmsg, -EACCES, 0);
614633

615634
kfree(pathbuf);
616635

security/integrity/ima/ima_main.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1051,7 +1051,7 @@ static int __init init_ima(void)
10511051
pr_warn("Couldn't register LSM notifier, error %d\n", error);
10521052

10531053
if (!error)
1054-
ima_update_policy_flag();
1054+
ima_update_policy_flags();
10551055

10561056
return error;
10571057
}

security/integrity/ima/ima_policy.c

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ int ima_policy_flag;
5353
static int temp_ima_appraise;
5454
static int build_ima_appraise __ro_after_init;
5555

56+
atomic_t ima_setxattr_allowed_hash_algorithms;
57+
5658
#define MAX_LSM_RULES 6
5759
enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE,
5860
LSM_SUBJ_USER, LSM_SUBJ_ROLE, LSM_SUBJ_TYPE
@@ -720,24 +722,57 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
720722
return action;
721723
}
722724

723-
/*
724-
* Initialize the ima_policy_flag variable based on the currently
725-
* loaded policy. Based on this flag, the decision to short circuit
726-
* out of a function or not call the function in the first place
727-
* can be made earlier.
725+
/**
726+
* ima_update_policy_flags() - Update global IMA variables
727+
*
728+
* Update ima_policy_flag and ima_setxattr_allowed_hash_algorithms
729+
* based on the currently loaded policy.
730+
*
731+
* With ima_policy_flag, the decision to short circuit out of a function
732+
* or not call the function in the first place can be made earlier.
733+
*
734+
* With ima_setxattr_allowed_hash_algorithms, the policy can restrict the
735+
* set of hash algorithms accepted when updating the security.ima xattr of
736+
* a file.
737+
*
738+
* Context: called after a policy update and at system initialization.
728739
*/
729-
void ima_update_policy_flag(void)
740+
void ima_update_policy_flags(void)
730741
{
731742
struct ima_rule_entry *entry;
743+
int new_policy_flag = 0;
732744

745+
rcu_read_lock();
733746
list_for_each_entry(entry, ima_rules, list) {
747+
/*
748+
* SETXATTR_CHECK rules do not implement a full policy check
749+
* because rule checking would probably have an important
750+
* performance impact on setxattr(). As a consequence, only one
751+
* SETXATTR_CHECK can be active at a given time.
752+
* Because we want to preserve that property, we set out to use
753+
* atomic_cmpxchg. Either:
754+
* - the atomic was non-zero: a setxattr hash policy is
755+
* already enforced, we do nothing
756+
* - the atomic was zero: no setxattr policy was set, enable
757+
* the setxattr hash policy
758+
*/
759+
if (entry->func == SETXATTR_CHECK) {
760+
atomic_cmpxchg(&ima_setxattr_allowed_hash_algorithms,
761+
0, entry->allowed_algos);
762+
/* SETXATTR_CHECK doesn't impact ima_policy_flag */
763+
continue;
764+
}
765+
734766
if (entry->action & IMA_DO_MASK)
735-
ima_policy_flag |= entry->action;
767+
new_policy_flag |= entry->action;
736768
}
769+
rcu_read_unlock();
737770

738771
ima_appraise |= (build_ima_appraise | temp_ima_appraise);
739772
if (!ima_appraise)
740-
ima_policy_flag &= ~IMA_APPRAISE;
773+
new_policy_flag &= ~IMA_APPRAISE;
774+
775+
ima_policy_flag = new_policy_flag;
741776
}
742777

743778
static int ima_appraise_flag(enum ima_hooks func)
@@ -903,7 +938,9 @@ void __init ima_init_policy(void)
903938
ARRAY_SIZE(critical_data_rules),
904939
IMA_DEFAULT_POLICY);
905940

906-
ima_update_policy_flag();
941+
atomic_set(&ima_setxattr_allowed_hash_algorithms, 0);
942+
943+
ima_update_policy_flags();
907944
}
908945

909946
/* Make sure we have a valid policy, at least containing some rules. */
@@ -943,7 +980,7 @@ void ima_update_policy(void)
943980
*/
944981
kfree(arch_policy_entry);
945982
}
946-
ima_update_policy_flag();
983+
ima_update_policy_flags();
947984

948985
/* Custom IMA policy has been loaded */
949986
ima_process_queued_keys();
@@ -1176,6 +1213,23 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
11761213
if (ima_rule_contains_lsm_cond(entry))
11771214
return false;
11781215

1216+
break;
1217+
case SETXATTR_CHECK:
1218+
/* any action other than APPRAISE is unsupported */
1219+
if (entry->action != APPRAISE)
1220+
return false;
1221+
1222+
/* SETXATTR_CHECK requires an appraise_algos parameter */
1223+
if (!(entry->flags & IMA_VALIDATE_ALGOS))
1224+
return false;
1225+
1226+
/*
1227+
* full policies are not supported, they would have too
1228+
* much of a performance impact
1229+
*/
1230+
if (entry->flags & ~(IMA_FUNC | IMA_VALIDATE_ALGOS))
1231+
return false;
1232+
11791233
break;
11801234
default:
11811235
return false;
@@ -1332,6 +1386,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
13321386
entry->func = KEY_CHECK;
13331387
else if (strcmp(args[0].from, "CRITICAL_DATA") == 0)
13341388
entry->func = CRITICAL_DATA;
1389+
else if (strcmp(args[0].from, "SETXATTR_CHECK") == 0)
1390+
entry->func = SETXATTR_CHECK;
13351391
else
13361392
result = -EINVAL;
13371393
if (!result)

0 commit comments

Comments
 (0)