Hi Brian and Sascha,
> From: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> Sent: Monday, July 22, 2024 3:49 PM
> To: Brian Norris <briannorris@xxxxxxxxxxxx>
> Cc: Francesco Dolcini <francesco@xxxxxxxxxx>; Kalle Valo <kvalo@xxxxxxxxxx>;
> linux-wireless@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Francesco
> Dolcini <francesco.dolcini@xxxxxxxxxxx>; David Lin <yu-hao.lin@xxxxxxx>
> Subject: [EXT] Re: [PATCH v2 2/2] wifi: mwifiex: add support for
> WPA-PSK-SHA256
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> 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@xxxxxxxxxxx>
> > > > > Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> > > > > ---
> > > > > 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.
>
Yes, for our private driver. It uses '|=" and can work for firmware of IW416 and IW61x.
For nxpwifi, it will follow mwifiex first and will be updated to use "|=" later.
> >
> > That would make sense to me, but I think that's in conflict with what
> > David Lin said here:
> >
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore
> > .kernel.org%2Fall%2FPA4PR04MB9638B7F0F4E49F79057C15FBD1CD2%40PA
> 4PR04MB
> >
> 9638.eurprd04.prod.outlook.com%2F&data=05%7C02%7Cyu-hao.lin%40nxp.co
> m%
> >
> 7C0bdf1446db20472a267008dcaa22c655%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1635%
> >
> 7C0%7C0%7C638572313533588836%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwM
> >
> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sd
> ata=p
> > oKXdL5f%2Bwp7uXaPqvl38IF9OS5XtRfir3xIyEoAV0E%3D&reserved=0
> >
> > "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.
>
Mwifiex supports a lot of legacy devices, I don't know if modifications of the coding
for the data of TLV_TYPE_UAP_AKMP will affect existed devices or not. Maybe you
can follow the patch for host mlme to add a flag like ''host_mlme_enabled'' to enable
this kind of change for specific device.
David
