Quoting Mimi Zohar (zohar@xxxxxxxxxxxxxxxxxx):
> From: Dmitry Kasatkin <dmitry.kasatkin@xxxxxxxxx>
>
> EVM protects a file's security extended attributes(xattrs) against integrity
> attacks. The current patchset maintains an HMAC-sha1 value across the security
> xattrs, storing the value as the extended attribute 'security.evm'. We
> anticipate other methods for protecting the security extended attributes.
> This patch reserves the first byte of 'security.evm' as a place holder for
> the type of method.
>
> Changelog v6:
> - move evm_ima_xattr_type definition to security/integrity/integrity.h
> - defined a structure for the EVM xattr called evm_ima_xattr_data
> (based on Serge Hallyn's suggestion)
>
> Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@xxxxxxxxx>
> Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxx>
> ---
> include/linux/integrity.h | 1 +
> security/integrity/evm/evm_crypto.c | 11 +++++++----
> security/integrity/evm/evm_main.c | 10 +++++-----
> security/integrity/integrity.h | 11 +++++++++++
> 4 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index e715a2a..9684433 100644
> --- a/include/linux/integrity.h
> +++ b/include/linux/integrity.h
> @@ -19,6 +19,7 @@ enum integrity_status {
> INTEGRITY_UNKNOWN,
> };
>
> +/* List of EVM protected security xattrs */
> #ifdef CONFIG_INTEGRITY
> extern int integrity_inode_alloc(struct inode *inode);
> extern void integrity_inode_free(struct inode *inode);
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index d49bb00..c631b99 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -141,14 +141,17 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
> const char *xattr_value, size_t xattr_value_len)
> {
> struct inode *inode = dentry->d_inode;
> - u8 hmac[SHA1_DIGEST_SIZE];
> + struct evm_ima_xattr_data xattr_data;
> int rc = 0;
>
> rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
> - xattr_value_len, hmac);
> - if (rc == 0)
> + xattr_value_len, xattr_data.digest);
> + if (rc == 0) {
> + xattr_data.type = EVM_XATTR_HMAC;
> rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_EVM,
> - hmac, SHA1_DIGEST_SIZE, 0);
> + &xattr_data,
> + sizeof(xattr_data), 0);
> + }
> else if (rc == -ENODATA)
> rc = inode->i_op->removexattr(dentry, XATTR_NAME_EVM);
> return rc;
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index a8fa45f..c0580dd1 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -51,20 +51,20 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
> size_t xattr_value_len,
> struct integrity_iint_cache *iint)
> {
> - char hmac_val[SHA1_DIGEST_SIZE];
> + struct evm_ima_xattr_data xattr_data;
> int rc;
>
> if (iint->hmac_status != INTEGRITY_UNKNOWN)
> return iint->hmac_status;
>
> - memset(hmac_val, 0, sizeof hmac_val);
Why did you drop the memset here?
(You didn't in the previous version of this patch)
Otherwise, looks good.
Acked-by: Serge Hallyn <serge.hallyn@xxxxxxxxxxxxx>
> rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
> - xattr_value_len, hmac_val);
> + xattr_value_len, xattr_data.digest);
> if (rc < 0)
> return INTEGRITY_UNKNOWN;
>
> - rc = vfs_xattr_cmp(dentry, XATTR_NAME_EVM, hmac_val, sizeof hmac_val,
> - GFP_NOFS);
> + xattr_data.type = EVM_XATTR_HMAC;
> + rc = vfs_xattr_cmp(dentry, XATTR_NAME_EVM, (u8 *)&xattr_data,
> + sizeof xattr_data, GFP_NOFS);
> if (rc < 0)
> goto err_out;
> iint->hmac_status = INTEGRITY_PASS;
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 397a46b..7efbf56 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -18,6 +18,17 @@
> /* iint cache flags */
> #define IMA_MEASURED 0x01
>
> +enum evm_ima_xattr_type {
> + IMA_XATTR_DIGEST = 0x01,
> + EVM_XATTR_HMAC,
> + EVM_IMA_XATTR_DIGSIG,
> +};
> +
> +struct evm_ima_xattr_data {
> + u8 type;
> + u8 digest[SHA1_DIGEST_SIZE];
> +} __attribute__((packed));
> +
> /* integrity data associated with an inode */
> struct integrity_iint_cache {
> struct rb_node rb_node; /* rooted in integrity_iint_tree */
> --
> 1.7.3.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
