linux-kernel - Re: [PATCH] ima: add the ability to query ima for the hash of a given file.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8f4d9c4e-735d-8ba9-b84a-4f341030e0cf@linux.microsoft.com>
Date:   Fri, 20 Dec 2019 08:48:20 -0800
From:   Lakshmi Ramasubramanian <nramas@...ux.microsoft.com>
To:     Florent Revest <revest@...omium.org>,
        linux-integrity@...r.kernel.org
Cc:     kpsingh@...omium.org, mjg59@...gle.com, zohar@...ux.ibm.com,
        linux-kernel@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        Florent Revest <revest@...gle.com>
Subject: Re: [PATCH] ima: add the ability to query ima for the hash of a given
 file.

On 12/20/2019 8:31 AM, Florent Revest wrote:

>   
> +/**
> + * ima_file_hash - return the stored measurement if a file has been hashed.
> + * @file: pointer to the file
> + * @buf: buffer in which to store the hash
> + * @buf_size: length of the buffer
> + *
> + * On success, output the hash into buf and return the hash algorithm (as
> + * defined in the enum hash_algo).

> + * If the hash is larger than buf, then only size bytes will be copied. It
> + * generally just makes sense to pass a buffer capable of holding the largest
> + * possible hash: IMA_MAX_DIGEST_SIZE

If the given buffer is smaller than the hash length, wouldn't it be 
better to return the required size and a status indicating the buffer is 
not enough. The caller can then call back with the required buffer.

If the hash is truncated the caller may not know if the hash is partial 
or not.

> + *
> + * If IMA is disabled or if no measurement is available, return -EOPNOTSUPP.
> + * If the parameters are incorrect, return -EINVAL.
> + */
> +int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> +{
> +	struct inode *inode;
> +	struct integrity_iint_cache *iint;
> +	size_t copied_size;
> +
> +	if (!file || !buf)
> +		return -EINVAL;
> +
> +	if (!ima_policy_flag)
> +		return -EOPNOTSUPP;
> +
> +	inode = file_inode(file);
> +	iint = integrity_iint_find(inode);
> +	if (!iint)
> +		return -EOPNOTSUPP;
> +
> +	mutex_lock(&iint->mutex);
> +	copied_size = min_t(size_t, iint->ima_hash->length, buf_size);
> +	memcpy(buf, iint->ima_hash->digest, copied_size);
> +	mutex_unlock(&iint->mutex);
> +
> +	return iint->ima_hash->algo;

Should the hash algorithm be copied from iinit->ima_hash to a local 
variable while holding the mutex and that one returned?

I assume iinit->mutex  is taken to ensure iinit->ima_hash is not removed 
while this function is accessing it.

thanks,
  -lakshmi