linux-kernel - Re: [PATCH] orinoco: Use shash instead of ahash for MIC calculations

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: <20161213075425.GA5948@zzz>
Date:   Mon, 12 Dec 2016 23:54:25 -0800
From:   Eric Biggers <ebiggers3@...il.com>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
        linux-wireless@...r.kernel.org, linux-crypto@...r.kernel.org,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Stephan Mueller <smueller@...onox.de>
Subject: Re: [PATCH] orinoco: Use shash instead of ahash for MIC calculations

On Mon, Dec 12, 2016 at 12:55:55PM -0800, Andy Lutomirski wrote:
> +int orinoco_mic(struct crypto_shash *tfm_michael, u8 *key,
>  		u8 *da, u8 *sa, u8 priority,
>  		u8 *data, size_t data_len, u8 *mic)
>  {
> -	AHASH_REQUEST_ON_STACK(req, tfm_michael);
> -	struct scatterlist sg[2];
> +	SHASH_DESC_ON_STACK(desc, tfm_michael);
>  	u8 hdr[ETH_HLEN + 2]; /* size of header + padding */
>  	int err;
>  
> @@ -67,18 +66,27 @@ int orinoco_mic(struct crypto_ahash *tfm_michael, u8 *key,
>  	hdr[ETH_ALEN * 2 + 2] = 0;
>  	hdr[ETH_ALEN * 2 + 3] = 0;
>  
> -	/* Use scatter gather to MIC header and data in one go */
> -	sg_init_table(sg, 2);
> -	sg_set_buf(&sg[0], hdr, sizeof(hdr));
> -	sg_set_buf(&sg[1], data, data_len);
> +	desc->tfm = tfm_michael;
> +	desc->flags = 0;
>  
> -	if (crypto_ahash_setkey(tfm_michael, key, MIC_KEYLEN))
> -		return -1;
> +	err = crypto_shash_setkey(tfm_michael, key, MIC_KEYLEN);
> +	if (err)
> +		return err;
> +
> +	err = crypto_shash_init(desc);
> +	if (err)
> +		return err;
> +
> +	err = crypto_shash_update(desc, hdr, sizeof(hdr));
> +	if (err)
> +		return err;
> +
> +	err = crypto_shash_update(desc, data, data_len);
> +	if (err)
> +		return err;
> +
> +	err = crypto_shash_final(desc, mic);
> +	shash_desc_zero(desc);
>  
> -	ahash_request_set_tfm(req, tfm_michael);
> -	ahash_request_set_callback(req, 0, NULL, NULL);
> -	ahash_request_set_crypt(req, sg, mic, data_len + sizeof(hdr));
> -	err = crypto_ahash_digest(req);
> -	ahash_request_zero(req);
>  	return err;

It's probably a good idea to always do shash_desc_zero(), even when something
above it fails.  Otherwise this looks fine.  Thanks for sending these patches!

Eric