Skip to content

Commit

Permalink
Merge branch 'restrict-digest-alg-v8' into next-integrity
Browse files Browse the repository at this point in the history
Taken from the cover letter "IMA: restrict the accepted digest
algorithms for the security.ima xattr":

Provide users the ability to restrict the algorithms accepted by
their system, both when writing/updating xattrs, and when appraising
files, while retaining a permissive behavior by default to preserve
backward compatibility.

To provide these features, alter the behavior of setxattr to
only accept hashes built in the kernel, instead of any hash listed
in the kernel (complete list crypto/hash_info.c). In addition, the
user can define in his IMA policy the list of digest algorithms
allowed for writing to the security.ima xattr. In that case,
only algorithms present in that list are accepted for writing.

In addition, users may opt-in to allowlist hash algorithms for
appraising thanks to the new 'appraise_algos' IMA policy option.
By default IMA will keep accepting any hash algorithm, but specifying
that option will make appraisal of files hashed with another algorithm
fail.

Link: https://lore.kernel.org/linux-integrity/20210816081056.24530-1-Simon.THOBY@viveris.fr/
  • Loading branch information
mimizohar committed Aug 18, 2021
2 parents e37be53 + 8ecd39c commit d07eeeb
Show file tree
Hide file tree
Showing 7 changed files with 268 additions and 35 deletions.
15 changes: 13 additions & 2 deletions Documentation/ABI/testing/ima_policy
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@ Description:
lsm: [[subj_user=] [subj_role=] [subj_type=]
[obj_user=] [obj_role=] [obj_type=]]
option: [[appraise_type=]] [template=] [permit_directio]
[appraise_flag=] [keyrings=]
[appraise_flag=] [appraise_algos=] [keyrings=]
base:
func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
[FIRMWARE_CHECK]
[FIRMWARE_CHECK]
[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
[KEXEC_CMDLINE] [KEY_CHECK] [CRITICAL_DATA]
[SETXATTR_CHECK]
mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
[[^]MAY_EXEC]
fsmagic:= hex value
Expand All @@ -55,6 +56,10 @@ Description:
label:= [selinux]|[kernel_info]|[data_label]
data_label:= a unique string used for grouping and limiting critical data.
For example, "selinux" to measure critical data for SELinux.
appraise_algos:= comma-separated list of hash algorithms
For example, "sha256,sha512" to only accept to appraise
files where the security.ima xattr was hashed with one
of these two algorithms.

default policy:
# PROC_SUPER_MAGIC
Expand Down Expand Up @@ -134,3 +139,9 @@ Description:
keys added to .builtin_trusted_keys or .ima keyring:

measure func=KEY_CHECK keyrings=.builtin_trusted_keys|.ima

Example of the special SETXATTR_CHECK appraise rule, that
restricts the hash algorithms allowed when writing to the
security.ima xattr of a file:

appraise func=SETXATTR_CHECK appraise_algos=sha256,sha384,sha512
1 change: 0 additions & 1 deletion security/integrity/ima/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ config IMA
select SECURITYFS
select CRYPTO
select CRYPTO_HMAC
select CRYPTO_MD5
select CRYPTO_SHA1
select CRYPTO_HASH_INFO
select TCG_TPM if HAS_IOMEM && !UML
Expand Down
14 changes: 9 additions & 5 deletions security/integrity/ima/ima.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,11 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
/* current content of the policy */
extern int ima_policy_flag;

/* bitset of digests algorithms allowed in the setxattr hook */
extern atomic_t ima_setxattr_allowed_hash_algorithms;

/* set during initialization */
extern int ima_hash_algo;
extern int ima_hash_algo __ro_after_init;
extern int ima_sha1_idx __ro_after_init;
extern int ima_hash_algo_idx __ro_after_init;
extern int ima_extra_slots __ro_after_init;
Expand Down Expand Up @@ -198,6 +201,7 @@ static inline unsigned int ima_hash_key(u8 *digest)
hook(KEXEC_CMDLINE, kexec_cmdline) \
hook(KEY_CHECK, key) \
hook(CRITICAL_DATA, critical_data) \
hook(SETXATTR_CHECK, setxattr_check) \
hook(MAX_CHECK, none)

#define __ima_hook_enumify(ENUM, str) ENUM,
Expand Down Expand Up @@ -254,7 +258,7 @@ int ima_get_action(struct user_namespace *mnt_userns, struct inode *inode,
const struct cred *cred, u32 secid, int mask,
enum ima_hooks func, int *pcr,
struct ima_template_desc **template_desc,
const char *func_data);
const char *func_data, unsigned int *allowed_algos);
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,
Expand Down Expand Up @@ -285,10 +289,10 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
const struct cred *cred, u32 secid, enum ima_hooks func,
int mask, int flags, int *pcr,
struct ima_template_desc **template_desc,
const char *func_data);
const char *func_data, unsigned int *allowed_algos);
void ima_init_policy(void);
void ima_update_policy(void);
void ima_update_policy_flag(void);
void ima_update_policy_flags(void);
ssize_t ima_parse_add_rule(char *);
void ima_delete_rules(void);
int ima_check_policy(void);
Expand Down Expand Up @@ -319,7 +323,7 @@ int ima_must_appraise(struct user_namespace *mnt_userns, struct inode *inode,
void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
enum ima_hooks func);
enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value,
int xattr_len);
int ima_read_xattr(struct dentry *dentry,
struct evm_ima_xattr_data **xattr_value);
Expand Down
6 changes: 4 additions & 2 deletions security/integrity/ima/ima_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
* @pcr: pointer filled in if matched measure policy sets pcr=
* @template_desc: pointer filled in if matched measure policy sets template=
* @func_data: func specific data, may be NULL
* @allowed_algos: allowlist of hash algorithms for the IMA xattr
*
* The policy is defined in terms of keypairs:
* subj=, obj=, type=, func=, mask=, fsmagic=
Expand All @@ -188,14 +189,15 @@ int ima_get_action(struct user_namespace *mnt_userns, struct inode *inode,
const struct cred *cred, u32 secid, int mask,
enum ima_hooks func, int *pcr,
struct ima_template_desc **template_desc,
const char *func_data)
const char *func_data, unsigned int *allowed_algos)
{
int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH;

flags &= ima_policy_flag;

return ima_match_policy(mnt_userns, inode, cred, secid, func, mask,
flags, pcr, template_desc, func_data);
flags, pcr, template_desc, func_data,
allowed_algos);
}

/*
Expand Down
73 changes: 68 additions & 5 deletions security/integrity/ima/ima_appraise.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,9 @@ int ima_must_appraise(struct user_namespace *mnt_userns, struct inode *inode,
return 0;

security_task_getsecid_subj(current, &secid);
return ima_match_policy(mnt_userns, inode, current_cred(), secid, func,
mask, IMA_APPRAISE | IMA_HASH, NULL, NULL, NULL);
return ima_match_policy(mnt_userns, inode, current_cred(), secid,
func, mask, IMA_APPRAISE | IMA_HASH, NULL,
NULL, NULL, NULL);
}

static int ima_fix_xattr(struct dentry *dentry,
Expand Down Expand Up @@ -171,7 +172,7 @@ static void ima_cache_flags(struct integrity_iint_cache *iint,
}
}

enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value,
int xattr_len)
{
struct signature_v2_hdr *sig;
Expand Down Expand Up @@ -575,6 +576,66 @@ static void ima_reset_appraise_flags(struct inode *inode, int digsig)
clear_bit(IMA_DIGSIG, &iint->atomic_flags);
}

/**
* validate_hash_algo() - Block setxattr with unsupported hash algorithms
* @dentry: object of the setxattr()
* @xattr_value: userland supplied xattr value
* @xattr_value_len: length of xattr_value
*
* The xattr value is mapped to its hash algorithm, and this algorithm
* must be built in the kernel for the setxattr to be allowed.
*
* Emit an audit message when the algorithm is invalid.
*
* Return: 0 on success, else an error.
*/
static int validate_hash_algo(struct dentry *dentry,
const struct evm_ima_xattr_data *xattr_value,
size_t xattr_value_len)
{
char *path = NULL, *pathbuf = NULL;
enum hash_algo xattr_hash_algo;
const char *errmsg = "unavailable-hash-algorithm";
unsigned int allowed_hashes;

xattr_hash_algo = ima_get_hash_algo(xattr_value, xattr_value_len);

allowed_hashes = atomic_read(&ima_setxattr_allowed_hash_algorithms);

if (allowed_hashes) {
/* success if the algorithm is allowed in the ima policy */
if (allowed_hashes & (1U << xattr_hash_algo))
return 0;

/*
* We use a different audit message when the hash algorithm
* is denied by a policy rule, instead of not being built
* in the kernel image
*/
errmsg = "denied-hash-algorithm";
} else {
if (likely(xattr_hash_algo == ima_hash_algo))
return 0;

/* allow any xattr using an algorithm built in the kernel */
if (crypto_has_alg(hash_algo_name[xattr_hash_algo], 0, 0))
return 0;
}

pathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
if (!pathbuf)
return -EACCES;

path = dentry_path(dentry, pathbuf, PATH_MAX);

integrity_audit_msg(AUDIT_INTEGRITY_DATA, d_inode(dentry), path,
"set_data", errmsg, -EACCES, 0);

kfree(pathbuf);

return -EACCES;
}

int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
const void *xattr_value, size_t xattr_value_len)
{
Expand All @@ -592,9 +653,11 @@ int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
digsig = (xvalue->type == EVM_XATTR_PORTABLE_DIGSIG);
}
if (result == 1 || evm_revalidate_status(xattr_name)) {
result = validate_hash_algo(dentry, xvalue, xattr_value_len);
if (result)
return result;

ima_reset_appraise_flags(d_backing_inode(dentry), digsig);
if (result == 1)
result = 0;
}
return result;
}
Expand Down
20 changes: 16 additions & 4 deletions security/integrity/ima/ima_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
int xattr_len = 0;
bool violation_check;
enum hash_algo hash_algo;
unsigned int allowed_algos = 0;

if (!ima_policy_flag || !S_ISREG(inode->i_mode))
return 0;
Expand All @@ -224,7 +225,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
* Included is the appraise submask.
*/
action = ima_get_action(file_mnt_user_ns(file), inode, cred, secid,
mask, func, &pcr, &template_desc, NULL);
mask, func, &pcr, &template_desc, NULL,
&allowed_algos);
violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) &&
(ima_policy_flag & IMA_MEASURE));
if (!action && !violation_check)
Expand Down Expand Up @@ -361,6 +363,16 @@ static int process_measurement(struct file *file, const struct cred *cred,

if ((file->f_flags & O_DIRECT) && (iint->flags & IMA_PERMIT_DIRECTIO))
rc = 0;

/* Ensure the digest was generated using an allowed algorithm */
if (rc == 0 && must_appraise && allowed_algos != 0 &&
(allowed_algos & (1U << hash_algo)) == 0) {
rc = -EACCES;

integrity_audit_msg(AUDIT_INTEGRITY_DATA, file_inode(file),
pathname, "collect_data",
"denied-hash-algorithm", rc, 0);
}
out_locked:
if ((mask & MAY_WRITE) && test_bit(IMA_DIGSIG, &iint->atomic_flags) &&
!(iint->flags & IMA_NEW_FILE))
Expand Down Expand Up @@ -438,7 +450,7 @@ int ima_file_mprotect(struct vm_area_struct *vma, unsigned long prot)
inode = file_inode(vma->vm_file);
action = ima_get_action(file_mnt_user_ns(vma->vm_file), inode,
current_cred(), secid, MAY_EXEC, MMAP_CHECK,
&pcr, &template, NULL);
&pcr, &template, NULL, NULL);

/* Is the mmap'ed file in policy? */
if (!(action & (IMA_MEASURE | IMA_APPRAISE_SUBMASK)))
Expand Down Expand Up @@ -896,7 +908,7 @@ int process_buffer_measurement(struct user_namespace *mnt_userns,
security_task_getsecid_subj(current, &secid);
action = ima_get_action(mnt_userns, inode, current_cred(),
secid, 0, func, &pcr, &template,
func_data);
func_data, NULL);
if (!(action & IMA_MEASURE) && !digest)
return -ENOENT;
}
Expand Down Expand Up @@ -1039,7 +1051,7 @@ static int __init init_ima(void)
pr_warn("Couldn't register LSM notifier, error %d\n", error);

if (!error)
ima_update_policy_flag();
ima_update_policy_flags();

return error;
}
Expand Down
Loading

0 comments on commit d07eeeb

Please sign in to comment.