Now that AES-CMAC has a library API, convert aes_s2v() to use it instead
of a "cmac(aes)" crypto_shash. The result is faster and simpler code.
It's also more reliable, since with the library the only step that can
fail is preparing the key. In contrast, crypto_shash_digest(),
crypto_shash_init(), crypto_shash_update(), and crypto_shash_final()
could all fail and return an errno value. aes_s2v() ignored these
errors, which was a bug. So that bug is fixed as well.
As part of this, change the prototype of aes_s2v() to take the raw key
directly instead of a prepared key. Its only two callers prepare a key
for each call, so it might as well be done directly in aes_s2v().
Since this removes the last dependency on the "cmac(aes)" crypto_shash
from mac80211, also remove the 'select CRYPTO_CMAC'.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
net/mac80211/Kconfig | 1 -
net/mac80211/fils_aead.c | 48 ++++++++++++++--------------------------
2 files changed, 17 insertions(+), 32 deletions(-)
diff --git a/net/mac80211/Kconfig b/net/mac80211/Kconfig
index 0afbe4f4f976..d6bc295e23a1 100644
--- a/net/mac80211/Kconfig
+++ b/net/mac80211/Kconfig
@@ -6,11 +6,10 @@ config MAC80211
select CRYPTO_LIB_AES_CBC_MACS
select CRYPTO_LIB_ARC4
select CRYPTO_AES
select CRYPTO_CCM
select CRYPTO_GCM
- select CRYPTO_CMAC
select CRC32
help
This option enables the hardware independent IEEE 802.11
networking stack.
diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c
index 912c46f74d24..d2f4a17eab99 100644
--- a/net/mac80211/fils_aead.c
+++ b/net/mac80211/fils_aead.c
@@ -2,17 +2,15 @@
/*
* FILS AEAD for (Re)Association Request/Response frames
* Copyright 2016, Qualcomm Atheros, Inc.
*/
-#include <crypto/aes.h>
-#include <crypto/hash.h>
+#include <crypto/aes-cbc-macs.h>
#include <crypto/skcipher.h>
#include <crypto/utils.h>
#include "ieee80211_i.h"
-#include "aes_cmac.h"
#include "fils_aead.h"
static void gf_mulx(u8 *pad)
{
u64 a = get_unaligned_be64(pad);
@@ -20,58 +18,63 @@ static void gf_mulx(u8 *pad)
put_unaligned_be64((a << 1) | (b >> 63), pad);
put_unaligned_be64((b << 1) ^ ((a >> 63) ? 0x87 : 0), pad + 8);
}
-static int aes_s2v(struct crypto_shash *tfm,
+static int aes_s2v(const u8 *in_key, size_t key_len,
size_t num_elem, const u8 *addr[], size_t len[], u8 *v)
{
u8 d[AES_BLOCK_SIZE], tmp[AES_BLOCK_SIZE] = {};
- SHASH_DESC_ON_STACK(desc, tfm);
+ struct aes_cmac_key key;
+ struct aes_cmac_ctx ctx;
size_t i;
+ int res;
- desc->tfm = tfm;
+ res = aes_cmac_preparekey(&key, in_key, key_len);
+ if (res)
+ return res;
/* D = AES-CMAC(K, <zero>) */
- crypto_shash_digest(desc, tmp, AES_BLOCK_SIZE, d);
+ aes_cmac(&key, tmp, AES_BLOCK_SIZE, d);
for (i = 0; i < num_elem - 1; i++) {
/* D = dbl(D) xor AES_CMAC(K, Si) */
gf_mulx(d); /* dbl */
- crypto_shash_digest(desc, addr[i], len[i], tmp);
+ aes_cmac(&key, addr[i], len[i], tmp);
crypto_xor(d, tmp, AES_BLOCK_SIZE);
}
- crypto_shash_init(desc);
+ aes_cmac_init(&ctx, &key);
if (len[i] >= AES_BLOCK_SIZE) {
/* len(Sn) >= 128 */
/* T = Sn xorend D */
- crypto_shash_update(desc, addr[i], len[i] - AES_BLOCK_SIZE);
+ aes_cmac_update(&ctx, addr[i], len[i] - AES_BLOCK_SIZE);
crypto_xor(d, addr[i] + len[i] - AES_BLOCK_SIZE,
AES_BLOCK_SIZE);
} else {
/* len(Sn) < 128 */
/* T = dbl(D) xor pad(Sn) */
gf_mulx(d); /* dbl */
crypto_xor(d, addr[i], len[i]);
d[len[i]] ^= 0x80;
}
/* V = AES-CMAC(K, T) */
- crypto_shash_finup(desc, d, AES_BLOCK_SIZE, v);
+ aes_cmac_update(&ctx, d, AES_BLOCK_SIZE);
+ aes_cmac_final(&ctx, v);
+ memzero_explicit(&key, sizeof(key));
return 0;
}
/* Note: addr[] and len[] needs to have one extra slot at the end. */
static int aes_siv_encrypt(const u8 *key, size_t key_len,
const u8 *plain, size_t plain_len,
size_t num_elem, const u8 *addr[],
size_t len[], u8 *out)
{
u8 v[AES_BLOCK_SIZE];
- struct crypto_shash *tfm;
struct crypto_skcipher *tfm2;
struct skcipher_request *req;
int res;
struct scatterlist src[1], dst[1];
u8 *tmp;
@@ -81,19 +84,11 @@ static int aes_siv_encrypt(const u8 *key, size_t key_len,
addr[num_elem] = plain;
len[num_elem] = plain_len;
num_elem++;
/* S2V */
-
- tfm = crypto_alloc_shash("cmac(aes)", 0, 0);
- if (IS_ERR(tfm))
- return PTR_ERR(tfm);
- /* K1 for S2V */
- res = crypto_shash_setkey(tfm, key, key_len);
- if (!res)
- res = aes_s2v(tfm, num_elem, addr, len, v);
- crypto_free_shash(tfm);
+ res = aes_s2v(key /* K1 */, key_len, num_elem, addr, len, v);
if (res)
return res;
/* Use a temporary buffer of the plaintext to handle need for
* overwriting this during AES-CTR.
@@ -144,11 +139,10 @@ static int aes_siv_encrypt(const u8 *key, size_t key_len,
static int aes_siv_decrypt(const u8 *key, size_t key_len,
const u8 *iv_crypt, size_t iv_c_len,
size_t num_elem, const u8 *addr[], size_t len[],
u8 *out)
{
- struct crypto_shash *tfm;
struct crypto_skcipher *tfm2;
struct skcipher_request *req;
struct scatterlist src[1], dst[1];
size_t crypt_len;
int res;
@@ -196,19 +190,11 @@ static int aes_siv_decrypt(const u8 *key, size_t key_len,
crypto_free_skcipher(tfm2);
if (res)
return res;
/* S2V */
-
- tfm = crypto_alloc_shash("cmac(aes)", 0, 0);
- if (IS_ERR(tfm))
- return PTR_ERR(tfm);
- /* K1 for S2V */
- res = crypto_shash_setkey(tfm, key, key_len);
- if (!res)
- res = aes_s2v(tfm, num_elem, addr, len, check);
- crypto_free_shash(tfm);
+ res = aes_s2v(key /* K1 */, key_len, num_elem, addr, len, check);
if (res)
return res;
if (memcmp(check, frame_iv, AES_BLOCK_SIZE) != 0)
return -EINVAL;
return 0;
--
2.53.0
On Wed, 2026-02-18 at 13:35 -0800, Eric Biggers wrote:
> Now that AES-CMAC has a library API, convert aes_s2v() to use it instead
> of a "cmac(aes)" crypto_shash. The result is faster and simpler code.
>
> It's also more reliable, since with the library the only step that can
> fail is preparing the key. In contrast, crypto_shash_digest(),
> crypto_shash_init(), crypto_shash_update(), and crypto_shash_final()
> could all fail and return an errno value. aes_s2v() ignored these
> errors, which was a bug. So that bug is fixed as well.
>
> As part of this, change the prototype of aes_s2v() to take the raw key
> directly instead of a prepared key. Its only two callers prepare a key
> for each call, so it might as well be done directly in aes_s2v().
>
> Since this removes the last dependency on the "cmac(aes)" crypto_shash
> from mac80211, also remove the 'select CRYPTO_CMAC'.
>
> -static int aes_s2v(struct crypto_shash *tfm,
> +static int aes_s2v(const u8 *in_key, size_t key_len,
> size_t num_elem, const u8 *addr[], size_t len[], u8 *v)
> {
> u8 d[AES_BLOCK_SIZE], tmp[AES_BLOCK_SIZE] = {};
> - SHASH_DESC_ON_STACK(desc, tfm);
> + struct aes_cmac_key key;
> + struct aes_cmac_ctx ctx;
> size_t i;
> + int res;
>
> - desc->tfm = tfm;
> + res = aes_cmac_preparekey(&key, in_key, key_len);
> + if (res)
> + return res;
Same here, maybe, technically, but also doesn't matter.
Acked-by: Johannes Berg <johannes@sipsolutions.net>
johannes
On Thu, Feb 19, 2026 at 12:01:14PM +0100, Johannes Berg wrote:
> On Wed, 2026-02-18 at 13:35 -0800, Eric Biggers wrote:
> > Now that AES-CMAC has a library API, convert aes_s2v() to use it instead
> > of a "cmac(aes)" crypto_shash. The result is faster and simpler code.
> >
> > It's also more reliable, since with the library the only step that can
> > fail is preparing the key. In contrast, crypto_shash_digest(),
> > crypto_shash_init(), crypto_shash_update(), and crypto_shash_final()
> > could all fail and return an errno value. aes_s2v() ignored these
> > errors, which was a bug. So that bug is fixed as well.
> >
> > As part of this, change the prototype of aes_s2v() to take the raw key
> > directly instead of a prepared key. Its only two callers prepare a key
> > for each call, so it might as well be done directly in aes_s2v().
> >
> > Since this removes the last dependency on the "cmac(aes)" crypto_shash
> > from mac80211, also remove the 'select CRYPTO_CMAC'.
> >
>
> > -static int aes_s2v(struct crypto_shash *tfm,
> > +static int aes_s2v(const u8 *in_key, size_t key_len,
> > size_t num_elem, const u8 *addr[], size_t len[], u8 *v)
> > {
> > u8 d[AES_BLOCK_SIZE], tmp[AES_BLOCK_SIZE] = {};
> > - SHASH_DESC_ON_STACK(desc, tfm);
> > + struct aes_cmac_key key;
> > + struct aes_cmac_ctx ctx;
> > size_t i;
> > + int res;
> >
> > - desc->tfm = tfm;
> > + res = aes_cmac_preparekey(&key, in_key, key_len);
> > + if (res)
> > + return res;
>
> Same here, maybe, technically, but also doesn't matter.
>
> Acked-by: Johannes Berg <johannes@sipsolutions.net>
>
> johannes
In this case aes_s2v() wouldn't otherwise be able to fail, so ignoring
the aes_cmac_preparekey() return value would indeed be a simplification.
However, since the key length isn't a compile-time constant here, we'd
have to rely on non-local validation, which isn't ideal.
To ignore the return value entirely I'd prefer a static_assert that the
length is equal to one of AES_KEYSIZE_*, which isn't possible here.
It's actually not clear to me where the length validation happens before
here. nl80211_associate() for example just copies the length from
userspace without validating it. ieee80211_mgd_assoc() only checks that
the length is at most FILS_MAX_KEK_LEN (64).
- Eric
On Thu, 2026-02-19 at 14:15 -0800, Eric Biggers wrote:
> > > -static int aes_s2v(struct crypto_shash *tfm,
> > > +static int aes_s2v(const u8 *in_key, size_t key_len,
> > > size_t num_elem, const u8 *addr[], size_t len[], u8 *v)
> > > {
> > > u8 d[AES_BLOCK_SIZE], tmp[AES_BLOCK_SIZE] = {};
> > > - SHASH_DESC_ON_STACK(desc, tfm);
> > > + struct aes_cmac_key key;
> > > + struct aes_cmac_ctx ctx;
> > > size_t i;
> > > + int res;
> > >
> > > - desc->tfm = tfm;
> > > + res = aes_cmac_preparekey(&key, in_key, key_len);
> > > + if (res)
> > > + return res;
> >
> > Same here, maybe, technically, but also doesn't matter.
> >
> > Acked-by: Johannes Berg <johannes@sipsolutions.net>
> >
> > johannes
>
> In this case aes_s2v() wouldn't otherwise be able to fail, so ignoring
> the aes_cmac_preparekey() return value would indeed be a simplification.
Right.
> However, since the key length isn't a compile-time constant here, we'd
> have to rely on non-local validation, which isn't ideal.
That's true.
> To ignore the return value entirely I'd prefer a static_assert that the
> length is equal to one of AES_KEYSIZE_*, which isn't possible here.
Indeed.
> It's actually not clear to me where the length validation happens before
> here. nl80211_associate() for example just copies the length from
> userspace without validating it. ieee80211_mgd_assoc() only checks that
> the length is at most FILS_MAX_KEK_LEN (64).
Oh, right, I forgot this was FILS and mixed it up with the previous
patch. We probably _should_ check it earlier there, but it won't be
local here either way, and we have the error paths already so that's
fine.
johannes
© 2016 - 2026 Red Hat, Inc.