[PATCH 0/2] mwifiex: add support for WPA-PSK-SHA256

[PATCH 0/2] mwifiex: add support for WPA-PSK-SHA256

[Sascha Hauer]

This series adds support for the WPA-PSK AKM suite with SHA256 as hashing
method (WPA-PSK-SHA256)

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (2):
      wifi: mwifiex: simplify WPA flags setting
      wifi: mwifiex: add support for WPA-PSK-SHA256

 drivers/net/wireless/marvell/mwifiex/fw.h      |  1 +
 drivers/net/wireless/marvell/mwifiex/uap_cmd.c | 30 +++++++++-----------------
 2 files changed, 11 insertions(+), 20 deletions(-)
---
base-commit: 0c3836482481200ead7b416ca80c68a29cfdaabd
change-id: 20240716-mwifiex-wpa-psk-sha256-2d2a9fc3b06b

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

[PATCH 1/2] wifi: mwifiex: simplify WPA flags setting

[Sascha Hauer]

The WPA flags setting only depends on the wpa_versions bitfield and not
on the AKM suite, so move it out of the switch/case to simplify the code
a bit. No functional change intended.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/wireless/marvell/mwifiex/uap_cmd.c | 27 +++++++-------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
index 491e366119096..8d0cd84d27698 100644
--- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
@@ -46,31 +46,18 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
 
 	bss_config->key_mgmt_operation |= KEY_MGMT_ON_HOST;
 
+	if (params->crypto.wpa_versions & NL80211_WPA_VERSION_1)
+		bss_config->protocol = PROTOCOL_WPA;
+	if (params->crypto.wpa_versions & NL80211_WPA_VERSION_2)
+		bss_config->protocol |= PROTOCOL_WPA2;
+
 	for (i = 0; i < params->crypto.n_akm_suites; i++) {
 		switch (params->crypto.akm_suites[i]) {
 		case WLAN_AKM_SUITE_8021X:
-			if (params->crypto.wpa_versions &
-			    NL80211_WPA_VERSION_1) {
-				bss_config->protocol = PROTOCOL_WPA;
-				bss_config->key_mgmt = KEY_MGMT_EAP;
-			}
-			if (params->crypto.wpa_versions &
-			    NL80211_WPA_VERSION_2) {
-				bss_config->protocol |= PROTOCOL_WPA2;
-				bss_config->key_mgmt = KEY_MGMT_EAP;
-			}
+			bss_config->key_mgmt = KEY_MGMT_EAP;
 			break;
 		case WLAN_AKM_SUITE_PSK:
-			if (params->crypto.wpa_versions &
-			    NL80211_WPA_VERSION_1) {
-				bss_config->protocol = PROTOCOL_WPA;
-				bss_config->key_mgmt = KEY_MGMT_PSK;
-			}
-			if (params->crypto.wpa_versions &
-			    NL80211_WPA_VERSION_2) {
-				bss_config->protocol |= PROTOCOL_WPA2;
-				bss_config->key_mgmt = KEY_MGMT_PSK;
-			}
+			bss_config->key_mgmt = KEY_MGMT_PSK;
 			break;
 		default:
 			break;

-- 
2.39.2

[PATCH 2/2] wifi: mwifiex: add support for WPA-PSK-SHA256

[Sascha Hauer]

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.

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 8d0cd84d27698..c07aec67aa0fe 100644
--- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
+++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
@@ -59,6 +59,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.2

Re: [PATCH 1/2] wifi: mwifiex: simplify WPA flags setting

[Francesco Dolcini]

Hello Sasha,
thanks for your patch.

On Tue, Jul 16, 2024 at 01:33:27PM +0200, Sascha Hauer wrote:
> The WPA flags setting only depends on the wpa_versions bitfield and not
> on the AKM suite, so move it out of the switch/case to simplify the code
> a bit. No functional change intended.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>

> ---
>  drivers/net/wireless/marvell/mwifiex/uap_cmd.c | 27 +++++++-------------------
>  1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> index 491e366119096..8d0cd84d27698 100644
> --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> @@ -46,31 +46,18 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
>  
>  	bss_config->key_mgmt_operation |= KEY_MGMT_ON_HOST;
>  
> +	if (params->crypto.wpa_versions & NL80211_WPA_VERSION_1)
> +		bss_config->protocol = PROTOCOL_WPA;
> +	if (params->crypto.wpa_versions & NL80211_WPA_VERSION_2)
> +		bss_config->protocol |= PROTOCOL_WPA2;
> +

I am wondering what happens if NL80211_WPA_VERSION_1 is not set, no idea
what's the content of bss_config->protocol ...

in any case this is not something that you introduced, but maybe worth
having a look?

Francesco

Re: [PATCH 2/2] wifi: mwifiex: add support for WPA-PSK-SHA256

[Francesco Dolcini]

Hello Sascha,

On Tue, Jul 16, 2024 at 01:33:28PM +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.

Do you have any more details on which chip/firmware you tested?
The change looks good, I am just wondering if there are reasons this
might create issue on some specific chip/firmware combination.

Francesco

Re: [PATCH 2/2] wifi: mwifiex: add support for WPA-PSK-SHA256

[Sascha Hauer]

Hi Francesco,

On Tue, Jul 16, 2024 at 09:57:31PM +0200, Francesco Dolcini wrote:
> Hello Sascha,
> 
> On Tue, Jul 16, 2024 at 01:33:28PM +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.
> 
> Do you have any more details on which chip/firmware you tested?
> The change looks good, I am just wondering if there are reasons this
> might create issue on some specific chip/firmware combination.

I have a IW416 with firmware 16.92.21.p119. The change itself is derived
from the downstream driver. The downstream driver also sets the
KEY_MGMT_PSK_SHA256 bit unconditionally for all chip/firmware
combinations so I think this change should be ok.

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 |

Re: [PATCH 1/2] wifi: mwifiex: simplify WPA flags setting

[Sascha Hauer]

On Tue, Jul 16, 2024 at 09:48:26PM +0200, Francesco Dolcini wrote:
> Hello Sasha,
> thanks for your patch.
> 
> On Tue, Jul 16, 2024 at 01:33:27PM +0200, Sascha Hauer wrote:
> > The WPA flags setting only depends on the wpa_versions bitfield and not
> > on the AKM suite, so move it out of the switch/case to simplify the code
> > a bit. No functional change intended.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> 
> > ---
> >  drivers/net/wireless/marvell/mwifiex/uap_cmd.c | 27 +++++++-------------------
> >  1 file changed, 7 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > index 491e366119096..8d0cd84d27698 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > @@ -46,31 +46,18 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
> >  
> >  	bss_config->key_mgmt_operation |= KEY_MGMT_ON_HOST;
> >  
> > +	if (params->crypto.wpa_versions & NL80211_WPA_VERSION_1)
> > +		bss_config->protocol = PROTOCOL_WPA;
> > +	if (params->crypto.wpa_versions & NL80211_WPA_VERSION_2)
> > +		bss_config->protocol |= PROTOCOL_WPA2;
> > +
> 
> I am wondering what happens if NL80211_WPA_VERSION_1 is not set, no idea
> what's the content of bss_config->protocol ...
> 
> in any case this is not something that you introduced, but maybe worth
> having a look?

bss_config is kzalloced in the only caller of mwifiex_set_secure_params(),
so bss_config->protocol is zero on entry.

It might be worth setting it to zero explicitly here to make the code
better understandable without having to look at the callers.

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 |

Re: [PATCH 1/2] wifi: mwifiex: simplify WPA flags setting

[Francesco Dolcini]

On Wed, Jul 17, 2024 at 08:26:22AM +0200, Sascha Hauer wrote:
> On Tue, Jul 16, 2024 at 09:48:26PM +0200, Francesco Dolcini wrote:
> > On Tue, Jul 16, 2024 at 01:33:27PM +0200, Sascha Hauer wrote:
> > > The WPA flags setting only depends on the wpa_versions bitfield and not
> > > on the AKM suite, so move it out of the switch/case to simplify the code
> > > a bit. No functional change intended.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > 
> > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > 
> > > ---
> > >  drivers/net/wireless/marvell/mwifiex/uap_cmd.c | 27 +++++++-------------------
> > >  1 file changed, 7 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > index 491e366119096..8d0cd84d27698 100644
> > > --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > @@ -46,31 +46,18 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
> > >  
> > >  	bss_config->key_mgmt_operation |= KEY_MGMT_ON_HOST;
> > >  
> > > +	if (params->crypto.wpa_versions & NL80211_WPA_VERSION_1)
> > > +		bss_config->protocol = PROTOCOL_WPA;
> > > +	if (params->crypto.wpa_versions & NL80211_WPA_VERSION_2)
> > > +		bss_config->protocol |= PROTOCOL_WPA2;
> > > +
> > 
> > I am wondering what happens if NL80211_WPA_VERSION_1 is not set, no idea
> > what's the content of bss_config->protocol ...
> > 
> > in any case this is not something that you introduced, but maybe worth
> > having a look?
> 
> bss_config is kzalloced in the only caller of mwifiex_set_secure_params(),
> so bss_config->protocol is zero on entry.
> 
> It might be worth setting it to zero explicitly here to make the code
> better understandable without having to look at the callers.

Thanks for looking into this, to me the change is fine as it is, up to
you if you want to improve it this way.

Francesco

Re: [PATCH 2/2] wifi: mwifiex: add support for WPA-PSK-SHA256

[Francesco Dolcini]

On Wed, Jul 17, 2024 at 08:12:16AM +0200, Sascha Hauer wrote:
> On Tue, Jul 16, 2024 at 09:57:31PM +0200, Francesco Dolcini wrote:
> > Hello Sascha,
> > 
> > On Tue, Jul 16, 2024 at 01:33:28PM +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.
> > 
> > Do you have any more details on which chip/firmware you tested?
> > The change looks good, I am just wondering if there are reasons this
> > might create issue on some specific chip/firmware combination.
> 
> I have a IW416 with firmware 16.92.21.p119. The change itself is derived
> from the downstream driver. The downstream driver also sets the
> KEY_MGMT_PSK_SHA256 bit unconditionally for all chip/firmware
> combinations so I think this change should be ok.

Fine for me

Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>

Francesco

Re: [PATCH 1/2] wifi: mwifiex: simplify WPA flags setting

[Sascha Hauer]

On Wed, Jul 17, 2024 at 10:07:23AM +0200, Francesco Dolcini wrote:
> On Wed, Jul 17, 2024 at 08:26:22AM +0200, Sascha Hauer wrote:
> > On Tue, Jul 16, 2024 at 09:48:26PM +0200, Francesco Dolcini wrote:
> > > On Tue, Jul 16, 2024 at 01:33:27PM +0200, Sascha Hauer wrote:
> > > > The WPA flags setting only depends on the wpa_versions bitfield and not
> > > > on the AKM suite, so move it out of the switch/case to simplify the code
> > > > a bit. No functional change intended.
> > > > 
> > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > 
> > > Reviewed-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> > > 
> > > > ---
> > > >  drivers/net/wireless/marvell/mwifiex/uap_cmd.c | 27 +++++++-------------------
> > > >  1 file changed, 7 insertions(+), 20 deletions(-)
> > > > 
> > > > diff --git a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > index 491e366119096..8d0cd84d27698 100644
> > > > --- a/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > +++ b/drivers/net/wireless/marvell/mwifiex/uap_cmd.c
> > > > @@ -46,31 +46,18 @@ int mwifiex_set_secure_params(struct mwifiex_private *priv,
> > > >  
> > > >  	bss_config->key_mgmt_operation |= KEY_MGMT_ON_HOST;
> > > >  
> > > > +	if (params->crypto.wpa_versions & NL80211_WPA_VERSION_1)
> > > > +		bss_config->protocol = PROTOCOL_WPA;
> > > > +	if (params->crypto.wpa_versions & NL80211_WPA_VERSION_2)
> > > > +		bss_config->protocol |= PROTOCOL_WPA2;
> > > > +
> > > 
> > > I am wondering what happens if NL80211_WPA_VERSION_1 is not set, no idea
> > > what's the content of bss_config->protocol ...
> > > 
> > > in any case this is not something that you introduced, but maybe worth
> > > having a look?
> > 
> > bss_config is kzalloced in the only caller of mwifiex_set_secure_params(),
> > so bss_config->protocol is zero on entry.
> > 
> > It might be worth setting it to zero explicitly here to make the code
> > better understandable without having to look at the callers.
> 
> Thanks for looking into this, to me the change is fine as it is, up to
> you if you want to improve it this way.

I just sent a v2 including this change.

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 |