linux-kernel - RE: [EXT] Re: [PATCH v2 2/2] wifi: mwifiex: add support for WPA-PSK-SHA256

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:
 <PA4PR04MB963817EE5EA2F6BAEDF22CC2D1A82@PA4PR04MB9638.eurprd04.prod.outlook.com>
Date: Mon, 22 Jul 2024 08:46:37 +0000
From: David Lin <yu-hao.lin@....com>
To: Sascha Hauer <s.hauer@...gutronix.de>, Brian Norris
	<briannorris@...omium.org>
CC: Francesco Dolcini <francesco@...cini.it>, Kalle Valo <kvalo@...nel.org>,
	"linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Francesco
 Dolcini <francesco.dolcini@...adex.com>
Subject: RE: [EXT] Re: [PATCH v2 2/2] wifi: mwifiex: add support for
 WPA-PSK-SHA256

Hi Brian and Sascha,

> From: Sascha Hauer <s.hauer@...gutronix.de>
> Sent: Monday, July 22, 2024 3:49 PM
> To: Brian Norris <briannorris@...omium.org>
> Cc: Francesco Dolcini <francesco@...cini.it>; Kalle Valo <kvalo@...nel.org>;
> linux-wireless@...r.kernel.org; linux-kernel@...r.kernel.org; Francesco
> Dolcini <francesco.dolcini@...adex.com>; David Lin <yu-hao.lin@....com>
> 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@...adex.com>
> > > > > Signed-off-by: Sascha Hauer <s.hauer@...gutronix.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.
> 

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