[PATCH] mac80211: Allocate a sync skcipher explicitly for FILS AEAD

[PATCH] mac80211: Allocate a sync skcipher explicitly for FILS AEAD

[Jouni Malinen]

The skcipher could have been of the async variant which may return from
skcipher_encrypt() with -EINPROGRESS after having queued the request.
The FILS AEAD implementation here does not have code for dealing with
that possibility, so allocate a sync cipher explicitly to avoid
potential issues with hardware accelerators.

This is based on the patch sent out by Ard.

Fixes: 39404feee691 ("mac80211: FILS AEAD protection for station mode association frames")
Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
---
 net/mac80211/fils_aead.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c
index e3bbe24..912f3e2c 100644
--- a/net/mac80211/fils_aead.c
+++ b/net/mac80211/fils_aead.c
@@ -192,7 +192,7 @@ static int aes_siv_encrypt(const u8 *key, size_t key_len,
 
 	/* CTR */
 
-	tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0);
+	tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC);
 	if (IS_ERR(tfm2)) {
 		kfree(tmp);
 		return PTR_ERR(tfm2);
@@ -251,7 +251,7 @@ static int aes_siv_decrypt(const u8 *key, size_t key_len,
 
 	/* CTR */
 
-	tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0);
+	tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, CRYPTO_ALG_ASYNC);
 	if (IS_ERR(tfm2))
 		return PTR_ERR(tfm2);
 	/* K2 for CTR */
-- 
2.7.4

Re: [PATCH] mac80211: Allocate a sync skcipher explicitly for FILS AEAD

[Johannes Berg]

Hi,

> The skcipher could have been of the async variant which may return
> from skcipher_encrypt() with -EINPROGRESS after having queued the
> request.
> The FILS AEAD implementation here does not have code for dealing with
> that possibility, so allocate a sync cipher explicitly to avoid
> potential issues with hardware accelerators.

> -	tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0);
> +	tfm2 = crypto_alloc_skcipher("ctr(aes)", 0,
> CRYPTO_ALG_ASYNC);

I'll apply this, after having found some code elsewhere that does
something similar, but I'll note that this is super confusing, since
the only documentation mentioning this flag says:

The mask flag restricts the type of cipher. The only allowed flag is
CRYPTO_ALG_ASYNC to restrict the cipher lookup function to
asynchronous ciphers. Usually, a caller provides a 0 for the mask flag.

(I have a vague feeling the first sentence was intended to be
documentation for the algorithm *implementation* specifying the flag,
and the second for a caller doing a lookup, or something strange?)

johannes

Re: [PATCH] mac80211: Allocate a sync skcipher explicitly for FILS AEAD

[Herbert Xu]

On Mon, Feb 06, 2017 at 07:54:37AM +0100, Johannes Berg wrote:
> Hi,
> 
> > The skcipher could have been of the async variant which may return
> > from skcipher_encrypt() with -EINPROGRESS after having queued the
> > request.
> > The FILS AEAD implementation here does not have code for dealing with
> > that possibility, so allocate a sync cipher explicitly to avoid
> > potential issues with hardware accelerators.
> 
> > -	tfm2 = crypto_alloc_skcipher("ctr(aes)", 0, 0);
> > +	tfm2 = crypto_alloc_skcipher("ctr(aes)", 0,
> > CRYPTO_ALG_ASYNC);
> 
> I'll apply this, after having found some code elsewhere that does
> something similar, but I'll note that this is super confusing, since
> the only documentation mentioning this flag says:
> 
> The mask flag restricts the type of cipher. The only allowed flag is
> CRYPTO_ALG_ASYNC to restrict the cipher lookup function to
> asynchronous ciphers. Usually, a caller provides a 0 for the mask flag.

The type and mask are used as follows when checking an algorithm:

	alg->type & mask == type & mask

So to request a synchronous algorithm (that is, one with the
CRYPTO_ALG_ASYNC bit set to zero), you would set type to 0 and
mask to CRYPTO_ALG_ASYNC.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] mac80211: Allocate a sync skcipher explicitly for FILS AEAD

[Johannes Berg]

> The type and mask are used as follows when checking an algorithm:
> 
> 	alg->type & mask == type & mask
> 
> So to request a synchronous algorithm (that is, one with the
> CRYPTO_ALG_ASYNC bit set to zero), you would set type to 0 and
> mask to CRYPTO_ALG_ASYNC.

Ah. Ok, that makes sense, thanks for the explanation.

johannes