This adds support for the WPA-PSK AKM suite with SHA256 as hashing
method (WPA-PSK-SHA256). Tested with a wpa_supplicant provided AP
using key_mgmt=WPA-PSK-SHA256.
Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/net/wireless/marvell/mwifiex/fw.h | 1 +
drivers/net/wireless/marvell/mwifiex/uap_cmd.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index 3adc447b715f6..1c76754b616ff 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -415,6 +415,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER {
#define KEY_MGMT_NONE 0x04
#define KEY_MGMT_PSK 0x02
#define KEY_MGMT_EAP 0x01
+#define KEY_MGMT_PSK_SHA256 0x100
#define CIPHER_TKIP 0x04
#define CIPHER_AES_CCMP 0x08
#define VALID_CIPHER_BITMAP 0x0c
diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
index 7f822660fd955..c055fdc7114ba 100644
--- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
@@ -60,6 +60,9 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
case WLAN_AKM_SUITE_PSK:
bss_config->key_mgmt = KEY_MGMT_PSK;
break;
+ case WLAN_AKM_SUITE_PSK_SHA256:
+ bss_config->key_mgmt = KEY_MGMT_PSK_SHA256;
+ break;
default:
break;
}
--
2.39.2Hi Sascha,
On Wed, Jul 17, 2024 at 10:30:08AM +0200, Sascha Hauer wrote:
> This adds support for the WPA-PSK AKM suite with SHA256 as hashing
> method (WPA-PSK-SHA256). Tested with a wpa_supplicant provided AP
> using key_mgmt=WPA-PSK-SHA256.
>
> Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> drivers/net/wireless/marvell/mwifiex/fw.h | 1 +
> drivers/net/wireless/marvell/mwifiex/uap_cmd.c | 3 +++
> 2 files changed, 4 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
> index 3adc447b715f6..1c76754b616ff 100644
> --- a/drivers/net/wireless/marvell/mwifiex/fw.h
> +++ b/drivers/net/wireless/marvell/mwifiex/fw.h
> @@ -415,6 +415,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER {
> #define KEY_MGMT_NONE 0x04
> #define KEY_MGMT_PSK 0x02
> #define KEY_MGMT_EAP 0x01
> +#define KEY_MGMT_PSK_SHA256 0x100
> #define CIPHER_TKIP 0x04
> #define CIPHER_AES_CCMP 0x08
> #define VALID_CIPHER_BITMAP 0x0c
> diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> index 7f822660fd955..c055fdc7114ba 100644
> --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> @@ -60,6 +60,9 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
> case WLAN_AKM_SUITE_PSK:
> bss_config->key_mgmt = KEY_MGMT_PSK;
> break;
> + case WLAN_AKM_SUITE_PSK_SHA256:
> + bss_config->key_mgmt = KEY_MGMT_PSK_SHA256;
> + break;
I feel like this relates to previous questions you've had [1], and while
I think the answer at the time made sense to me (basically, EAP and PSK
are mutually exclusive), it makes less sense to me here that PSK-SHA256
is mutually exclusive with PSK. And in particular, IIUC, this means that
the ordering in a wpa_supplicant.conf line like
key_mgmt=WPA-PSK WPA-PSK-SHA256
matters -- only the latter will actually be in use.
Is that intended? Is this really a single-value field, and not a
multiple-option bitfield?
Or if these are really mutually exclusive, then maybe we're on the wrong
track here:
https://patchwork.kernel.org/project/linux-wireless/patch/20240530130156.1651174-1-s.hauer@pengutronix.de/
wifi: mwifiex: increase max_num_akm_suites
In any case, something feels off here, because the nl80211 API doesn't
say anything about the ordering of AKM suites being relevant.
Brian
> default:
> break;
> }
>
> --
> 2.39.2
>
[1] Subject: Re: [EXT] Re: [PATCH v10 2/2] wifi: mwifiex: add host mlme for AP mode
https://lore.kernel.org/all/Zmvjw3aG9j8kW0Ld@pengutronix.de/
https://lore.kernel.org/all/PA4PR04MB9638B7F0F4E49F79057C15FBD1CD2@PA4PR04MB9638.eurprd04.prod.outlook.com/
On Thu, Jul 18, 2024 at 03:55:18PM -0700, Brian Norris wrote:
> Hi Sascha,
>
> On Wed, Jul 17, 2024 at 10:30:08AM +0200, Sascha Hauer wrote:
> > This adds support for the WPA-PSK AKM suite with SHA256 as hashing
> > method (WPA-PSK-SHA256). Tested with a wpa_supplicant provided AP
> > using key_mgmt=WPA-PSK-SHA256.
> >
> > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> > drivers/net/wireless/marvell/mwifiex/fw.h | 1 +
> > drivers/net/wireless/marvell/mwifiex/uap_cmd.c | 3 +++
> > 2 files changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
> > index 3adc447b715f6..1c76754b616ff 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/fw.h
> > +++ b/drivers/net/wireless/marvell/mwifiex/fw.h
> > @@ -415,6 +415,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER {
> > #define KEY_MGMT_NONE 0x04
> > #define KEY_MGMT_PSK 0x02
> > #define KEY_MGMT_EAP 0x01
> > +#define KEY_MGMT_PSK_SHA256 0x100
> > #define CIPHER_TKIP 0x04
> > #define CIPHER_AES_CCMP 0x08
> > #define VALID_CIPHER_BITMAP 0x0c
> > diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > index 7f822660fd955..c055fdc7114ba 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > @@ -60,6 +60,9 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
> > case WLAN_AKM_SUITE_PSK:
> > bss_config->key_mgmt = KEY_MGMT_PSK;
> > break;
> > + case WLAN_AKM_SUITE_PSK_SHA256:
> > + bss_config->key_mgmt = KEY_MGMT_PSK_SHA256;
> > + break;
>
> I feel like this relates to previous questions you've had [1], and while
> I think the answer at the time made sense to me (basically, EAP and PSK
> are mutually exclusive), it makes less sense to me here that PSK-SHA256
> is mutually exclusive with PSK. And in particular, IIUC, this means that
> the ordering in a wpa_supplicant.conf line like
>
> key_mgmt=WPA-PSK WPA-PSK-SHA256
>
> matters -- only the latter will actually be in use.
>
> Is that intended? Is this really a single-value field, and not a
> multiple-option bitfield?
It seems that when only the KEY_MGMT_PSK_SHA256 is set, then
KEY_MGMT_PSK also works. Likewise, when only KEY_MGMT_SAE is set, then
also KEY_MGMT_PSK_SHA256 and KEY_MGMT_PSK work.
I gave it a test and also was surprised to see that we only have to set
the "most advanced" bit which then includes the "less advanced" features
automatically.
I could change setting the key_mgmt bits to |= as it feels more natural
and raises less eyebrows, but in my testing it didn't make a difference.
BTW wpa_supplicant parses the key_mgmt options into a bitfield which is
then evaluated elsewhere, so the order the AKM suites are passed to the
kernel is always the same, regardless of the order they appear in the
config.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[ +CC David, in case he has thoughts ]
On Fri, Jul 19, 2024 at 08:04:59AM +0200, Sascha Hauer wrote:
> On Thu, Jul 18, 2024 at 03:55:18PM -0700, Brian Norris wrote:
> > On Wed, Jul 17, 2024 at 10:30:08AM +0200, Sascha Hauer wrote:
> > > This adds support for the WPA-PSK AKM suite with SHA256 as hashing
> > > method (WPA-PSK-SHA256). Tested with a wpa_supplicant provided AP
> > > using key_mgmt=WPA-PSK-SHA256.
> > >
> > > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > ---
> > > drivers/net/wireless/marvell/mwifiex/fw.h | 1 +
> > > drivers/net/wireless/marvell/mwifiex/uap_cmd.c | 3 +++
> > > 2 files changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
> > > index 3adc447b715f6..1c76754b616ff 100644
> > > --- a/drivers/net/wireless/marvell/mwifiex/fw.h
> > > +++ b/drivers/net/wireless/marvell/mwifiex/fw.h
> > > @@ -415,6 +415,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER {
> > > #define KEY_MGMT_NONE 0x04
> > > #define KEY_MGMT_PSK 0x02
> > > #define KEY_MGMT_EAP 0x01
> > > +#define KEY_MGMT_PSK_SHA256 0x100
> > > #define CIPHER_TKIP 0x04
> > > #define CIPHER_AES_CCMP 0x08
> > > #define VALID_CIPHER_BITMAP 0x0c
> > > diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > index 7f822660fd955..c055fdc7114ba 100644
> > > --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > @@ -60,6 +60,9 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
> > > case WLAN_AKM_SUITE_PSK:
> > > bss_config->key_mgmt = KEY_MGMT_PSK;
> > > break;
> > > + case WLAN_AKM_SUITE_PSK_SHA256:
> > > + bss_config->key_mgmt = KEY_MGMT_PSK_SHA256;
> > > + break;
> >
> > I feel like this relates to previous questions you've had [1], and while
> > I think the answer at the time made sense to me (basically, EAP and PSK
> > are mutually exclusive), it makes less sense to me here that PSK-SHA256
> > is mutually exclusive with PSK. And in particular, IIUC, this means that
> > the ordering in a wpa_supplicant.conf line like
> >
> > key_mgmt=WPA-PSK WPA-PSK-SHA256
> >
> > matters -- only the latter will actually be in use.
> >
> > Is that intended? Is this really a single-value field, and not a
> > multiple-option bitfield?
>
> It seems that when only the KEY_MGMT_PSK_SHA256 is set, then
> KEY_MGMT_PSK also works. Likewise, when only KEY_MGMT_SAE is set, then
> also KEY_MGMT_PSK_SHA256 and KEY_MGMT_PSK work.
> I gave it a test and also was surprised to see that we only have to set
> the "most advanced" bit which then includes the "less advanced" features
> automatically.
Huh, that's interesting. So these KEY_MGMT* flags don't really mean what
they say. It might be nice to have some additional commentary in the
driver in that case.
> I could change setting the key_mgmt bits to |= as it feels more natural
> and raises less eyebrows, but in my testing it didn't make a difference.
That would make sense to me, but I think that's in conflict with what
David Lin said here:
https://lore.kernel.org/all/PA4PR04MB9638B7F0F4E49F79057C15FBD1CD2@PA4PR04MB9638.eurprd04.prod.outlook.com/
"Firmware can only support one of WLAN_AKM_SUITE_8021X,
WLAN_AKM_SUITE_PSK, or WLAN_AKM_SUITE_SAE."
If that's true, then it seems like we need some kind of priority
conditions here (e.g., if PSK is provided, but then we see PSK_SHA256,
let PSK_SHA256 override -- but not vice versa). That might be pretty
ugly though.
> BTW wpa_supplicant parses the key_mgmt options into a bitfield which is
> then evaluated elsewhere, so the order the AKM suites are passed to the
> kernel is always the same, regardless of the order they appear in the
> config.
I hear you, but that's not really how we define kernel APIs -- by the
particular implementation of a single commonly-used user space.
Brian
On Fri, Jul 19, 2024 at 12:05:01PM -0700, Brian Norris wrote:
> [ +CC David, in case he has thoughts ]
>
> On Fri, Jul 19, 2024 at 08:04:59AM +0200, Sascha Hauer wrote:
> > On Thu, Jul 18, 2024 at 03:55:18PM -0700, Brian Norris wrote:
> > > On Wed, Jul 17, 2024 at 10:30:08AM +0200, Sascha Hauer wrote:
> > > > This adds support for the WPA-PSK AKM suite with SHA256 as hashing
> > > > method (WPA-PSK-SHA256). Tested with a wpa_supplicant provided AP
> > > > using key_mgmt=WPA-PSK-SHA256.
> > > >
> > > > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > ---
> > > > drivers/net/wireless/marvell/mwifiex/fw.h | 1 +
> > > > drivers/net/wireless/marvell/mwifiex/uap_cmd.c | 3 +++
> > > > 2 files changed, 4 insertions(+)
> > > >
> > > > diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
> > > > index 3adc447b715f6..1c76754b616ff 100644
> > > > --- a/drivers/net/wireless/marvell/mwifiex/fw.h
> > > > +++ b/drivers/net/wireless/marvell/mwifiex/fw.h
> > > > @@ -415,6 +415,7 @@ enum MWIFIEX_802_11_PRIVACY_FILTER {
> > > > #define KEY_MGMT_NONE 0x04
> > > > #define KEY_MGMT_PSK 0x02
> > > > #define KEY_MGMT_EAP 0x01
> > > > +#define KEY_MGMT_PSK_SHA256 0x100
> > > > #define CIPHER_TKIP 0x04
> > > > #define CIPHER_AES_CCMP 0x08
> > > > #define VALID_CIPHER_BITMAP 0x0c
> > > > diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > index 7f822660fd955..c055fdc7114ba 100644
> > > > --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > @@ -60,6 +60,9 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
> > > > case WLAN_AKM_SUITE_PSK:
> > > > bss_config->key_mgmt = KEY_MGMT_PSK;
> > > > break;
> > > > + case WLAN_AKM_SUITE_PSK_SHA256:
> > > > + bss_config->key_mgmt = KEY_MGMT_PSK_SHA256;
> > > > + break;
> > >
> > > I feel like this relates to previous questions you've had [1], and while
> > > I think the answer at the time made sense to me (basically, EAP and PSK
> > > are mutually exclusive), it makes less sense to me here that PSK-SHA256
> > > is mutually exclusive with PSK. And in particular, IIUC, this means that
> > > the ordering in a wpa_supplicant.conf line like
> > >
> > > key_mgmt=WPA-PSK WPA-PSK-SHA256
> > >
> > > matters -- only the latter will actually be in use.
> > >
> > > Is that intended? Is this really a single-value field, and not a
> > > multiple-option bitfield?
> >
> > It seems that when only the KEY_MGMT_PSK_SHA256 is set, then
> > KEY_MGMT_PSK also works. Likewise, when only KEY_MGMT_SAE is set, then
> > also KEY_MGMT_PSK_SHA256 and KEY_MGMT_PSK work.
> > I gave it a test and also was surprised to see that we only have to set
> > the "most advanced" bit which then includes the "less advanced" features
> > automatically.
>
> Huh, that's interesting. So these KEY_MGMT* flags don't really mean what
> they say. It might be nice to have some additional commentary in the
> driver in that case.
>
> > I could change setting the key_mgmt bits to |= as it feels more natural
> > and raises less eyebrows, but in my testing it didn't make a difference.
Thinking about this again we really do need to use '|=' and not '='
to make the result independent of the ordering of the AKM suites array
entries.
>
> That would make sense to me, but I think that's in conflict with what
> David Lin said here:
>
> https://lore.kernel.org/all/PA4PR04MB9638B7F0F4E49F79057C15FBD1CD2@PA4PR04MB9638.eurprd04.prod.outlook.com/
>
> "Firmware can only support one of WLAN_AKM_SUITE_8021X,
> WLAN_AKM_SUITE_PSK, or WLAN_AKM_SUITE_SAE."
I don't really know how this sentence was meant. It clearly works when
both WLAN_AKM_SUITE_PSK and WLAN_AKM_SUITE_SAE are advertised. Of course
in the only one of both is selected by the station.
>
> If that's true, then it seems like we need some kind of priority
> conditions here (e.g., if PSK is provided, but then we see PSK_SHA256,
> let PSK_SHA256 override -- but not vice versa). That might be pretty
> ugly though.
>
> > BTW wpa_supplicant parses the key_mgmt options into a bitfield which is
> > then evaluated elsewhere, so the order the AKM suites are passed to the
> > kernel is always the same, regardless of the order they appear in the
> > config.
>
> I hear you, but that's not really how we define kernel APIs -- by the
> particular implementation of a single commonly-used user space.
I know. I wrote this just as a note that we can't use wpa_supplicant out
of the box to actually test how a different order of AKM suites in the
array behaves.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On Mon, Jul 22, 2024 at 09:48:59AM +0200, Sascha Hauer wrote: > On Fri, Jul 19, 2024 at 12:05:01PM -0700, Brian Norris wrote: > > On Fri, Jul 19, 2024 at 08:04:59AM +0200, Sascha Hauer wrote: > > > I could change setting the key_mgmt bits to |= as it feels more natural > > > and raises less eyebrows, but in my testing it didn't make a difference. > Thinking about this again we really do need to use '|=' and not '=' > to make the result independent of the ordering of the AKM suites array > entries. Yes, absolutely. I missed this while looking at your changes. Francesco
© 2016 - 2025 Red Hat, Inc.