Re: [PATCH/proposal] dm-crypt: add digest-based iv generation mode

[Matt Mackall <mpm@selenic.com>]

On Sat, Feb 21, 2004 at 03:17:25AM +0100, Christophe Saout wrote:
> 
> And I'm memcpy'ing the tfm onto the stack now to have a local copy.
> So I don't need to lock and it makes HMAC precomputation possible
> because I don't destroy the original tfm context.
> 
> +static int crypt_iv_hmac(struct crypt_config *cc, u8 *iv, sector_t sector)
> +{
> +	struct scatterlist sg = {
> +		.page = virt_to_page(iv),
> +		.offset = offset_in_page(iv),
> +		.length = sizeof(u64) / sizeof(u8)
> +	};
> +	int i;
> +	int tfm_size = sizeof(*cc->digest) + cc->digest->__crt_alg->cra_ctxsize;
> +	char tfm[tfm_size];
> +
> +	*(u64 *)iv = cpu_to_le64((u64)sector);
> +
> +	/* HMAC has already been initialized, finish it on private copy */
> +	memcpy(tfm, cc->digest, tfm_size);

As this stands, it's rather scary.

- it will quietly break when cryptoapi gets fiddled with
- it subverts the module reference counting rules
- for a given cipher/digest, tfm_size might be large

Subverting the API this way is bad. On the other hand, I tend to think
the API does need a way to deal with problem cases like these, so I'd
support extending the API in some fashion to handle it. Related (but
not identical) issues have cropped up with a few other things that
want to avoid serializing around a single or per-cpu context.

Something like:

 /* calculate the size of a tfm so that users can manage their own
 copies */

 int crypto_alg_size(const char *name);

 /* copy a TFM to a user-managed buffer, possibly on stack, with proper
 internal reference counting and any other necessary magic, size checks
 against boneheaded buffer sizing */

 crypto_copy_tfm(char *dst, const struct crypto_tfm *src, int size);

 /* do all the necessary bookkeeping to release a user-managed TFM, use
 char pointer to avoid alloc/free mismatch */

 crypto_copy_cleanup_tfm(char *usertfm);


James, thoughts?

-- 
Matt Mackall : http://www.selenic.com : Linux development and consulting