Hello Matthew,
I have just a few minor nits. Feel free to ignore if you don't think
it's worth addressing them.
Matthew Garrett <mjg59@xxxxxxxxxx> writes:
> @@ -203,10 +205,24 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> if (!(inode->i_opflags & IOP_XATTR))
> return -EOPNOTSUPP;
>
> - desc = init_desc(type);
> + desc = init_desc(type, hash_algo);
> if (IS_ERR(desc))
> return PTR_ERR(desc);
>
> + /*
> + * HMACs are always SHA1, but signatures may be of varying sizes, so
> + * allocate a buffer for the non-HMAC case and return the size to
> + * the caller
> + */
> + if (type != EVM_XATTR_HMAC) {
> + *digestlen = crypto_shash_digestsize(desc->tfm);
> + *digest = kmalloc(*digestlen, GFP_KERNEL);
> + if (!*digest) {
> + error = -ENOMEM;
> + goto out;
> + }
> + }
> +
> error = -ENODATA;
> for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
> bool is_ima = false;
I think it's worth mentioning either here or in the comment at the
beginning of this function that when type == EVM_XATTR_HMAC *digest is
expected to point to an existing buffer.
> @@ -251,18 +267,18 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
>
> int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
> const char *req_xattr_value, size_t req_xattr_value_len,
> - char *digest)
> + char **digest)
IMHO it's a slightly better interface if evm_calc_hmac() keeps taking a
char *, since it won't need to change where its argument points to.
> {
> return evm_calc_hmac_or_hash(dentry, req_xattr_name, req_xattr_value,
> - req_xattr_value_len, EVM_XATTR_HMAC, digest);
> + req_xattr_value_len, EVM_XATTR_HMAC, 0x02, digest, NULL);
It's clearer to use HASH_ALGO_SHA1 instead of 0x02 here.
> }
> @@ -333,7 +349,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr,
> {
> struct shash_desc *desc;
>
> - desc = init_desc(EVM_XATTR_HMAC);
> + desc = init_desc(EVM_XATTR_HMAC, 0x02);
Same here.
> if (IS_ERR(desc)) {
> pr_info("init_desc failed\n");
> return PTR_ERR(desc);
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 9ea9c19a545c..3d952e69852f 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -28,6 +28,8 @@
> #include <crypto/algapi.h>
> #include "evm.h"
>
> +#define HMAC_LEN 20
It's clearer to use SHA1_DIGEST_SIZE instead of 20 here.
> +
> int evm_initialized;
>
> static const char * const integrity_status_msg[] = {
--
Thiago Jung Bauermann
IBM Linux Technology Center
