.../broadcom/brcm80211/brcmfmac/cfg80211.c | 46 +++++++++------------- .../broadcom/brcm80211/brcmfmac/fwil_types.h | 2 +- 2 files changed, 20 insertions(+), 28 deletions(-)
Using the WSEC command instead of sae_password seems to be the supported
mechanism on newer firmware, and also how the brcmdhd driver does it.
According to user reports [1], the sae_password codepath doesn't actually
work on machines with Cypress chips anyway, so no harm in removing it.
This makes WPA3 work with iwd, or with wpa_supplicant pending a support
patchset [2].
[1] https://rachelbythebay.com/w/2023/11/06/wpa3/
[2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html
Signed-off-by: Hector Martin <marcan@marcan.st>
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 46 +++++++++-------------
.../broadcom/brcm80211/brcmfmac/fwil_types.h | 2 +-
2 files changed, 20 insertions(+), 28 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 2a90bb24ba77..138af70a33b8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -1687,52 +1687,44 @@ static u16 brcmf_map_fw_linkdown_reason(const struct brcmf_event_msg *e)
return reason;
}
-static int brcmf_set_pmk(struct brcmf_if *ifp, const u8 *pmk_data, u16 pmk_len)
+static int brcmf_set_wsec(struct brcmf_if *ifp, const u8 *key, u16 key_len, u16 flags)
{
struct brcmf_pub *drvr = ifp->drvr;
struct brcmf_wsec_pmk_le pmk;
int err;
+ if (key_len > sizeof(pmk.key)) {
+ bphy_err(drvr, "key must be less than %zu bytes\n",
+ sizeof(pmk.key));
+ return -EINVAL;
+ }
+
memset(&pmk, 0, sizeof(pmk));
- /* pass pmk directly */
- pmk.key_len = cpu_to_le16(pmk_len);
- pmk.flags = cpu_to_le16(0);
- memcpy(pmk.key, pmk_data, pmk_len);
+ /* pass key material directly */
+ pmk.key_len = cpu_to_le16(key_len);
+ pmk.flags = cpu_to_le16(flags);
+ memcpy(pmk.key, key, key_len);
- /* store psk in firmware */
+ /* store key material in firmware */
err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SET_WSEC_PMK,
&pmk, sizeof(pmk));
if (err < 0)
bphy_err(drvr, "failed to change PSK in firmware (len=%u)\n",
- pmk_len);
+ key_len);
return err;
}
+static int brcmf_set_pmk(struct brcmf_if *ifp, const u8 *pmk_data, u16 pmk_len)
+{
+ return brcmf_set_wsec(ifp, pmk_data, pmk_len, 0);
+}
+
static int brcmf_set_sae_password(struct brcmf_if *ifp, const u8 *pwd_data,
u16 pwd_len)
{
- struct brcmf_pub *drvr = ifp->drvr;
- struct brcmf_wsec_sae_pwd_le sae_pwd;
- int err;
-
- if (pwd_len > BRCMF_WSEC_MAX_SAE_PASSWORD_LEN) {
- bphy_err(drvr, "sae_password must be less than %d\n",
- BRCMF_WSEC_MAX_SAE_PASSWORD_LEN);
- return -EINVAL;
- }
-
- sae_pwd.key_len = cpu_to_le16(pwd_len);
- memcpy(sae_pwd.key, pwd_data, pwd_len);
-
- err = brcmf_fil_iovar_data_set(ifp, "sae_password", &sae_pwd,
- sizeof(sae_pwd));
- if (err < 0)
- bphy_err(drvr, "failed to set SAE password in firmware (len=%u)\n",
- pwd_len);
-
- return err;
+ return brcmf_set_wsec(ifp, pwd_data, pwd_len, BRCMF_WSEC_PASSPHRASE);
}
static void brcmf_link_down(struct brcmf_cfg80211_vif *vif, u16 reason,
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
index 611d1a6aabb9..b68c46caabe8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
@@ -584,7 +584,7 @@ struct brcmf_wsec_key_le {
struct brcmf_wsec_pmk_le {
__le16 key_len;
__le16 flags;
- u8 key[2 * BRCMF_WSEC_MAX_PSK_LEN + 1];
+ u8 key[BRCMF_WSEC_MAX_SAE_PASSWORD_LEN];
};
/**
---
base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa
change-id: 20231107-brcmfmac-wpa3-9e5f66e8be34
Best regards,
--
Hector Martin <marcan@marcan.st>Hey Hector, On Tue, Nov 07, 2023 at 03:05:31PM +0900, Hector Martin wrote: > Using the WSEC command instead of sae_password seems to be the supported > mechanism on newer firmware, and also how the brcmdhd driver does it. > > According to user reports [1], the sae_password codepath doesn't actually > work on machines with Cypress chips anyway, so no harm in removing it. I'm sorry to disappoint you but I've just tested this patch on a "Pinebook Pro" which has AP6255 module and it broke WPA3 Personal. No error messages are emitted to the kernel log, just iwctl saying it can't establish connection. This is using "Cypress" firmware from the Linux firmware tree [0] renamed to "brcmfmac43455-sdio.bin" which has the following features (extracted from last two lines): 43455c0-roml/43455_sdio-pno-aoe-pktfilter-pktctx-wfds-mfp-dfsradar-wowlpf-idsup-idauth-noclminc-clm_min-obss-obssdump-swdiv-gtkoe-roamprof-txbf-ve-sae-dpp-sr-okc-bpd Version: 7.45.234 (4ca95bb CY) CRC: 212e223d Date: Thu 2021-04-15 03:06:00 PDT Ucode Ver: 1043.2161 FWID 01-996384e2 DVID 01-1fda2915 This module is used on many SBCs, including some RaspberryPi boards. The reason RaspberryPi owners complain about lack of WPA3 Personal support is that most of them are using obscure downstream distros which ship brcmfmac firmware from somewhere else rather than the Linux firmware tree, so they lack the "sae" feature. Another is that it only works with iwd while default is wpa_supplicant. So far all known reports of those who tried the right firmware on RaspberryPi boards confirm WPA3 Personal was working with iwd [1]. I'll be happy to do more testing if needed. Thank you very much for your hard and insightful work! [0] https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/plain/cypress/cyfmac43455-sdio.bin [1] https://github.com/raspberrypi/linux/issues/4718#issuecomment-1279951709 -- Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software! mailto:fercerpav@gmail.com
On 2023/12/20 19:16, Paul Fertser wrote: > Hey Hector, > > On Tue, Nov 07, 2023 at 03:05:31PM +0900, Hector Martin wrote: >> Using the WSEC command instead of sae_password seems to be the supported >> mechanism on newer firmware, and also how the brcmdhd driver does it. >> >> According to user reports [1], the sae_password codepath doesn't actually >> work on machines with Cypress chips anyway, so no harm in removing it. > > I'm sorry to disappoint you but I've just tested this patch on a > "Pinebook Pro" which has AP6255 module and it broke WPA3 Personal. > > No error messages are emitted to the kernel log, just iwctl saying it > can't establish connection. > > This is using "Cypress" firmware from the Linux firmware tree [0] > renamed to "brcmfmac43455-sdio.bin" which has the following features > (extracted from last two lines): > > 43455c0-roml/43455_sdio-pno-aoe-pktfilter-pktctx-wfds-mfp-dfsradar-wowlpf-idsup-idauth-noclminc-clm_min-obss-obssdump-swdiv-gtkoe-roamprof-txbf-ve-sae-dpp-sr-okc-bpd Version: 7.45.234 (4ca95bb CY) CRC: 212e223d Date: Thu 2021-04-15 03:06:00 PDT Ucode Ver: 1043.2161 FWID 01-996384e2 > DVID 01-1fda2915 > > > This module is used on many SBCs, including some RaspberryPi > boards. The reason RaspberryPi owners complain about lack of WPA3 > Personal support is that most of them are using obscure downstream > distros which ship brcmfmac firmware from somewhere else rather than > the Linux firmware tree, so they lack the "sae" feature. Another is > that it only works with iwd while default is wpa_supplicant. > > So far all known reports of those who tried the right firmware on > RaspberryPi boards confirm WPA3 Personal was working with iwd [1]. > > > I'll be happy to do more testing if needed. Thank you very much for > your hard and insightful work! Thank you for being the first person to actually test any of this :) Now we actually have a reason to keep the code. The next thing I wonder is whether any of the *other* Cypress chips will respond to WSEC (in addition to or instead of sae_password)... Are you willing to test all the other wifi stuff we have queued up downstream? There's a whole pile of changes here: https://github.com/AsahiLinux/linux/commits/bits/080-wifi/ If things break it would be very helpful if you could bisect it down to the specific commit. This patch is also in there of course, feel free to revert/rebase it out. - Hector
Hector Martin <marcan@marcan.st> wrote: > Using the WSEC command instead of sae_password seems to be the supported > mechanism on newer firmware, and also how the brcmdhd driver does it. > > According to user reports [1], the sae_password codepath doesn't actually > work on machines with Cypress chips anyway, so no harm in removing it. > > This makes WPA3 work with iwd, or with wpa_supplicant pending a support > patchset [2]. > > [1] https://rachelbythebay.com/w/2023/11/06/wpa3/ > [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html > > Signed-off-by: Hector Martin <marcan@marcan.st> > Reviewed-by: Neal Gompa <neal@gompa.dev> Arend, what do you think? We recently talked about people testing brcmfmac patches, has anyone else tested this? -- https://patchwork.kernel.org/project/linux-wireless/patch/20231107-brcmfmac-wpa3-v1-1-4c7db8636680@marcan.st/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On December 17, 2023 12:25:23 PM Kalle Valo <kvalo@kernel.org> wrote: > Hector Martin <marcan@marcan.st> wrote: > >> Using the WSEC command instead of sae_password seems to be the supported >> mechanism on newer firmware, and also how the brcmdhd driver does it. >> >> According to user reports [1], the sae_password codepath doesn't actually >> work on machines with Cypress chips anyway, so no harm in removing it. >> >> This makes WPA3 work with iwd, or with wpa_supplicant pending a support >> patchset [2]. >> >> [1] https://rachelbythebay.com/w/2023/11/06/wpa3/ >> [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html >> >> Signed-off-by: Hector Martin <marcan@marcan.st> >> Reviewed-by: Neal Gompa <neal@gompa.dev> > > Arend, what do you think? > > We recently talked about people testing brcmfmac patches, has anyone else > tested this? Not sure I already replied so maybe I am repeating myself. I would prefer to keep the Cypress sae_password path as well although it reportedly does not work. The vendor support in the driver can be used to accommodate for that. The other option would be to have people with Cypress chipset test this patch. If that works for both we can consider dropping the sae_password path. Regards, Arend > -- > https://patchwork.kernel.org/project/linux-wireless/patch/20231107-brcmfmac-wpa3-v1-1-4c7db8636680@marcan.st/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 2023/12/19 17:52, Arend Van Spriel wrote: > On December 17, 2023 12:25:23 PM Kalle Valo <kvalo@kernel.org> wrote: > >> Hector Martin <marcan@marcan.st> wrote: >> >>> Using the WSEC command instead of sae_password seems to be the supported >>> mechanism on newer firmware, and also how the brcmdhd driver does it. >>> >>> According to user reports [1], the sae_password codepath doesn't actually >>> work on machines with Cypress chips anyway, so no harm in removing it. >>> >>> This makes WPA3 work with iwd, or with wpa_supplicant pending a support >>> patchset [2]. >>> >>> [1] https://rachelbythebay.com/w/2023/11/06/wpa3/ >>> [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html >>> >>> Signed-off-by: Hector Martin <marcan@marcan.st> >>> Reviewed-by: Neal Gompa <neal@gompa.dev> >> >> Arend, what do you think? >> >> We recently talked about people testing brcmfmac patches, has anyone else >> tested this? > > Not sure I already replied so maybe I am repeating myself. I would prefer > to keep the Cypress sae_password path as well although it reportedly does > not work. The vendor support in the driver can be used to accommodate for > that. The other option would be to have people with Cypress chipset test > this patch. If that works for both we can consider dropping the > sae_password path. > > Regards, > Arend So, if nobody from Cypress chimes in ever, and nobody cares nor tests Cypress chipsets, are we keeping any and all existing Cypress code-paths as bitrotting code forever and adding gratuitous conditionals every time any functionality needs to change "just in case it breaks Cypress" even though it has been tested compatible on Broadcom chipsets/firmware? Because that's not sustainable long term. - Hector
On 12/19/2023 12:01 PM, Hector Martin wrote: > > > On 2023/12/19 17:52, Arend Van Spriel wrote: >> On December 17, 2023 12:25:23 PM Kalle Valo <kvalo@kernel.org> wrote: >> >>> Hector Martin <marcan@marcan.st> wrote: >>> >>>> Using the WSEC command instead of sae_password seems to be the supported >>>> mechanism on newer firmware, and also how the brcmdhd driver does it. >>>> >>>> According to user reports [1], the sae_password codepath doesn't actually >>>> work on machines with Cypress chips anyway, so no harm in removing it. >>>> >>>> This makes WPA3 work with iwd, or with wpa_supplicant pending a support >>>> patchset [2]. >>>> >>>> [1] https://rachelbythebay.com/w/2023/11/06/wpa3/ >>>> [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html >>>> >>>> Signed-off-by: Hector Martin <marcan@marcan.st> >>>> Reviewed-by: Neal Gompa <neal@gompa.dev> >>> >>> Arend, what do you think? >>> >>> We recently talked about people testing brcmfmac patches, has anyone else >>> tested this? >> >> Not sure I already replied so maybe I am repeating myself. I would prefer >> to keep the Cypress sae_password path as well although it reportedly does >> not work. The vendor support in the driver can be used to accommodate for >> that. The other option would be to have people with Cypress chipset test >> this patch. If that works for both we can consider dropping the >> sae_password path. >> >> Regards, >> Arend > > So, if nobody from Cypress chimes in ever, and nobody cares nor tests > Cypress chipsets, are we keeping any and all existing Cypress code-paths > as bitrotting code forever and adding gratuitous conditionals every time > any functionality needs to change "just in case it breaks Cypress" even > though it has been tested compatible on Broadcom chipsets/firmware? > > Because that's not sustainable long term. You should look into WEXT just for the fun of it. If it were up to me and a bunch of other people that would have been gone decades ago. Maybe a bad example if the sae_password is indeed not working, but the Cypress chipset is used in RPi3 and RPi4 so there must be a couple of users. Regards, Arend Regards, Arend
Hi Arend and Kalle, On Wed, Dec 20, 2023 at 12:47 AM Arend van Spriel <arend.vanspriel@broadcom.com> wrote: > > On 12/19/2023 12:01 PM, Hector Martin wrote: > > > > > > On 2023/12/19 17:52, Arend Van Spriel wrote: > >> On December 17, 2023 12:25:23 PM Kalle Valo <kvalo@kernel.org> wrote: > >> > >>> Hector Martin <marcan@marcan.st> wrote: > >>> > >>>> Using the WSEC command instead of sae_password seems to be the supported > >>>> mechanism on newer firmware, and also how the brcmdhd driver does it. > >>>> > >>>> According to user reports [1], the sae_password codepath doesn't actually > >>>> work on machines with Cypress chips anyway, so no harm in removing it. > >>>> > >>>> This makes WPA3 work with iwd, or with wpa_supplicant pending a support > >>>> patchset [2]. > >>>> > >>>> [1] https://rachelbythebay.com/w/2023/11/06/wpa3/ > >>>> [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html > >>>> > >>>> Signed-off-by: Hector Martin <marcan@marcan.st> > >>>> Reviewed-by: Neal Gompa <neal@gompa.dev> > >>> > >>> Arend, what do you think? > >>> > >>> We recently talked about people testing brcmfmac patches, has anyone else > >>> tested this? > >> > >> Not sure I already replied so maybe I am repeating myself. I would prefer > >> to keep the Cypress sae_password path as well although it reportedly does > >> not work. The vendor support in the driver can be used to accommodate for > >> that. The other option would be to have people with Cypress chipset test > >> this patch. If that works for both we can consider dropping the > >> sae_password path. > >> > >> Regards, > >> Arend > > > > So, if nobody from Cypress chimes in ever, and nobody cares nor tests > > Cypress chipsets, are we keeping any and all existing Cypress code-paths > > as bitrotting code forever and adding gratuitous conditionals every time > > any functionality needs to change "just in case it breaks Cypress" even > > though it has been tested compatible on Broadcom chipsets/firmware? > > > > Because that's not sustainable long term. > > You should look into WEXT just for the fun of it. If it were up to me > and a bunch of other people that would have been gone decades ago. Maybe > a bad example if the sae_password is indeed not working, but the Cypress > chipset is used in RPi3 and RPi4 so there must be a couple of users. There are reports that WPA3 is broken on the Cypress chipsets the Raspberry Pis are using and this patch fixes it: https://rachelbythebay.com/w/2023/11/06/wpa3/ Based on that, it appears that all known users of WPA3 capable hardware with this driver require this fix. Thanks, -- Julian Calaby Email: julian.calaby@gmail.com Profile: http://www.google.com/profiles/julian.calaby/
Hi Julian, >>>>>> Using the WSEC command instead of sae_password seems to be the supported >>>>>> mechanism on newer firmware, and also how the brcmdhd driver does it. >>>>>> >>>>>> According to user reports [1], the sae_password codepath doesn't actually >>>>>> work on machines with Cypress chips anyway, so no harm in removing it. >>>>>> >>>>>> This makes WPA3 work with iwd, or with wpa_supplicant pending a support >>>>>> patchset [2]. >>>>>> >>>>>> [1] https://rachelbythebay.com/w/2023/11/06/wpa3/ >>>>>> [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html >>>>>> >>>>>> Signed-off-by: Hector Martin <marcan@marcan.st> >>>>>> Reviewed-by: Neal Gompa <neal@gompa.dev> >>>>> >>>>> Arend, what do you think? >>>>> >>>>> We recently talked about people testing brcmfmac patches, has anyone else >>>>> tested this? >>>> >>>> Not sure I already replied so maybe I am repeating myself. I would prefer >>>> to keep the Cypress sae_password path as well although it reportedly does >>>> not work. The vendor support in the driver can be used to accommodate for >>>> that. The other option would be to have people with Cypress chipset test >>>> this patch. If that works for both we can consider dropping the >>>> sae_password path. >>>> >>>> Regards, >>>> Arend >>> >>> So, if nobody from Cypress chimes in ever, and nobody cares nor tests >>> Cypress chipsets, are we keeping any and all existing Cypress code-paths >>> as bitrotting code forever and adding gratuitous conditionals every time >>> any functionality needs to change "just in case it breaks Cypress" even >>> though it has been tested compatible on Broadcom chipsets/firmware? >>> >>> Because that's not sustainable long term. >> >> You should look into WEXT just for the fun of it. If it were up to me >> and a bunch of other people that would have been gone decades ago. Maybe >> a bad example if the sae_password is indeed not working, but the Cypress >> chipset is used in RPi3 and RPi4 so there must be a couple of users. > > There are reports that WPA3 is broken on the Cypress chipsets the > Raspberry Pis are using and this patch fixes it: > https://rachelbythebay.com/w/2023/11/06/wpa3/ > > Based on that, it appears that all known users of WPA3 capable > hardware with this driver require this fix. the Pis are all using an outdated firmware. In their distro they put the firmware already under the alternates systems, but it just lacks the SAE offload support that is required to make WPA3 work. The linux-firmware version does the trick nicely. I documented what I did to make this work on Pi5 (note that I normally use Fedora on Pi4 and thus never encountered this issue) https://holtmann.dev/enabling-wpa3-on-raspberry-pi/ However you need to use iwd and not hope that you get a wpa_supplicant released version that will work. So whole game of wpa_supplicant is vendor specific to the company that provides the driver is also insane, but that is another story. Use iwd and you can most likely have WPA3 support if you have the right firmware. Regards Marcel
On Thu, Dec 21, 2023 at 3:40 PM Marcel Holtmann <marcel@holtmann.org> wrote: > > Hi Julian, > > >>>>>> Using the WSEC command instead of sae_password seems to be the supported > >>>>>> mechanism on newer firmware, and also how the brcmdhd driver does it. > >>>>>> > >>>>>> According to user reports [1], the sae_password codepath doesn't actually > >>>>>> work on machines with Cypress chips anyway, so no harm in removing it. > >>>>>> > >>>>>> This makes WPA3 work with iwd, or with wpa_supplicant pending a support > >>>>>> patchset [2]. > >>>>>> > >>>>>> [1] https://rachelbythebay.com/w/2023/11/06/wpa3/ > >>>>>> [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html > >>>>>> > >>>>>> Signed-off-by: Hector Martin <marcan@marcan.st> > >>>>>> Reviewed-by: Neal Gompa <neal@gompa.dev> > >>>>> > >>>>> Arend, what do you think? > >>>>> > >>>>> We recently talked about people testing brcmfmac patches, has anyone else > >>>>> tested this? > >>>> > >>>> Not sure I already replied so maybe I am repeating myself. I would prefer > >>>> to keep the Cypress sae_password path as well although it reportedly does > >>>> not work. The vendor support in the driver can be used to accommodate for > >>>> that. The other option would be to have people with Cypress chipset test > >>>> this patch. If that works for both we can consider dropping the > >>>> sae_password path. > >>>> > >>>> Regards, > >>>> Arend > >>> > >>> So, if nobody from Cypress chimes in ever, and nobody cares nor tests > >>> Cypress chipsets, are we keeping any and all existing Cypress code-paths > >>> as bitrotting code forever and adding gratuitous conditionals every time > >>> any functionality needs to change "just in case it breaks Cypress" even > >>> though it has been tested compatible on Broadcom chipsets/firmware? > >>> > >>> Because that's not sustainable long term. > >> > >> You should look into WEXT just for the fun of it. If it were up to me > >> and a bunch of other people that would have been gone decades ago. Maybe > >> a bad example if the sae_password is indeed not working, but the Cypress > >> chipset is used in RPi3 and RPi4 so there must be a couple of users. > > > > There are reports that WPA3 is broken on the Cypress chipsets the > > Raspberry Pis are using and this patch fixes it: > > https://rachelbythebay.com/w/2023/11/06/wpa3/ > > > > Based on that, it appears that all known users of WPA3 capable > > hardware with this driver require this fix. > > the Pis are all using an outdated firmware. In their distro they put the > firmware already under the alternates systems, but it just lacks the SAE > offload support that is required to make WPA3 work. The linux-firmware > version does the trick nicely. > > I documented what I did to make this work on Pi5 (note that I normally > use Fedora on Pi4 and thus never encountered this issue) > > https://holtmann.dev/enabling-wpa3-on-raspberry-pi/ > > However you need to use iwd and not hope that you get a wpa_supplicant > released version that will work. > > So whole game of wpa_supplicant is vendor specific to the company that > provides the driver is also insane, but that is another story. Use iwd > and you can most likely have WPA3 support if you have the right firmware. > wpa_supplicant is perfectly fine if the necessary patches are backported, as Fedora has done: https://src.fedoraproject.org/rpms/wpa_supplicant/c/99f4bf2096d3976cee01c499d7a30c1376f5f0f7 -- 真実はいつも一つ！/ Always, there's only one truth!
On 12/22/2023 1:03 AM, Neal Gompa wrote: > On Thu, Dec 21, 2023 at 3:40 PM Marcel Holtmann <marcel@holtmann.org> wrote: >> >> Hi Julian, >> >>>>>>>> Using the WSEC command instead of sae_password seems to be the supported >>>>>>>> mechanism on newer firmware, and also how the brcmdhd driver does it. >>>>>>>> >>>>>>>> According to user reports [1], the sae_password codepath doesn't actually >>>>>>>> work on machines with Cypress chips anyway, so no harm in removing it. >>>>>>>> >>>>>>>> This makes WPA3 work with iwd, or with wpa_supplicant pending a support >>>>>>>> patchset [2]. >>>>>>>> >>>>>>>> [1] https://rachelbythebay.com/w/2023/11/06/wpa3/ >>>>>>>> [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html >>>>>>>> >>>>>>>> Signed-off-by: Hector Martin <marcan@marcan.st> >>>>>>>> Reviewed-by: Neal Gompa <neal@gompa.dev> >>>>>>> >>>>>>> Arend, what do you think? >>>>>>> >>>>>>> We recently talked about people testing brcmfmac patches, has anyone else >>>>>>> tested this? >>>>>> >>>>>> Not sure I already replied so maybe I am repeating myself. I would prefer >>>>>> to keep the Cypress sae_password path as well although it reportedly does >>>>>> not work. The vendor support in the driver can be used to accommodate for >>>>>> that. The other option would be to have people with Cypress chipset test >>>>>> this patch. If that works for both we can consider dropping the >>>>>> sae_password path. >>>>>> >>>>>> Regards, >>>>>> Arend >>>>> >>>>> So, if nobody from Cypress chimes in ever, and nobody cares nor tests >>>>> Cypress chipsets, are we keeping any and all existing Cypress code-paths >>>>> as bitrotting code forever and adding gratuitous conditionals every time >>>>> any functionality needs to change "just in case it breaks Cypress" even >>>>> though it has been tested compatible on Broadcom chipsets/firmware? >>>>> >>>>> Because that's not sustainable long term. >>>> >>>> You should look into WEXT just for the fun of it. If it were up to me >>>> and a bunch of other people that would have been gone decades ago. Maybe >>>> a bad example if the sae_password is indeed not working, but the Cypress >>>> chipset is used in RPi3 and RPi4 so there must be a couple of users. >>> >>> There are reports that WPA3 is broken on the Cypress chipsets the >>> Raspberry Pis are using and this patch fixes it: >>> https://rachelbythebay.com/w/2023/11/06/wpa3/ >>> >>> Based on that, it appears that all known users of WPA3 capable >>> hardware with this driver require this fix. >> >> the Pis are all using an outdated firmware. In their distro they put the >> firmware already under the alternates systems, but it just lacks the SAE >> offload support that is required to make WPA3 work. The linux-firmware >> version does the trick nicely. >> >> I documented what I did to make this work on Pi5 (note that I normally >> use Fedora on Pi4 and thus never encountered this issue) >> >> https://holtmann.dev/enabling-wpa3-on-raspberry-pi/ >> >> However you need to use iwd and not hope that you get a wpa_supplicant >> released version that will work. >> >> So whole game of wpa_supplicant is vendor specific to the company that >> provides the driver is also insane, but that is another story. Use iwd >> and you can most likely have WPA3 support if you have the right firmware. >> > > wpa_supplicant is perfectly fine if the necessary patches are > backported, as Fedora has done: > https://src.fedoraproject.org/rpms/wpa_supplicant/c/99f4bf2096d3976cee01c499d7a30c1376f5f0f7 The brcmfmac firmware has its own 802.11 stack implementation and as such it has a SME running in firmware which means the driver only implements the NL80211_CMD_CONNECT primitive. Now if the firmware also has in-driver supplicant (*-idsup-*) supporting SAE (*-sae-*) it can be offloaded. That is what Cypress went with at least for upstream. For firmware without these in the firmware target string the driver needs to implement support for NL80211_CMD_EXTERNAL_AUTH, which is what we opted for in Broadcom BCA (or WCC-Access as we call it these days). So I don't think it is a fair assessment to call the wpa_supplicant implementation vendor specific. Regards, Arend
Hi Neal, >>>>>>>> Using the WSEC command instead of sae_password seems to be the supported >>>>>>>> mechanism on newer firmware, and also how the brcmdhd driver does it. >>>>>>>> >>>>>>>> According to user reports [1], the sae_password codepath doesn't actually >>>>>>>> work on machines with Cypress chips anyway, so no harm in removing it. >>>>>>>> >>>>>>>> This makes WPA3 work with iwd, or with wpa_supplicant pending a support >>>>>>>> patchset [2]. >>>>>>>> >>>>>>>> [1] https://rachelbythebay.com/w/2023/11/06/wpa3/ >>>>>>>> [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html >>>>>>>> >>>>>>>> Signed-off-by: Hector Martin <marcan@marcan.st> >>>>>>>> Reviewed-by: Neal Gompa <neal@gompa.dev> >>>>>>> >>>>>>> Arend, what do you think? >>>>>>> >>>>>>> We recently talked about people testing brcmfmac patches, has anyone else >>>>>>> tested this? >>>>>> >>>>>> Not sure I already replied so maybe I am repeating myself. I would prefer >>>>>> to keep the Cypress sae_password path as well although it reportedly does >>>>>> not work. The vendor support in the driver can be used to accommodate for >>>>>> that. The other option would be to have people with Cypress chipset test >>>>>> this patch. If that works for both we can consider dropping the >>>>>> sae_password path. >>>>>> >>>>>> Regards, >>>>>> Arend >>>>> >>>>> So, if nobody from Cypress chimes in ever, and nobody cares nor tests >>>>> Cypress chipsets, are we keeping any and all existing Cypress code-paths >>>>> as bitrotting code forever and adding gratuitous conditionals every time >>>>> any functionality needs to change "just in case it breaks Cypress" even >>>>> though it has been tested compatible on Broadcom chipsets/firmware? >>>>> >>>>> Because that's not sustainable long term. >>>> >>>> You should look into WEXT just for the fun of it. If it were up to me >>>> and a bunch of other people that would have been gone decades ago. Maybe >>>> a bad example if the sae_password is indeed not working, but the Cypress >>>> chipset is used in RPi3 and RPi4 so there must be a couple of users. >>> >>> There are reports that WPA3 is broken on the Cypress chipsets the >>> Raspberry Pis are using and this patch fixes it: >>> https://rachelbythebay.com/w/2023/11/06/wpa3/ >>> >>> Based on that, it appears that all known users of WPA3 capable >>> hardware with this driver require this fix. >> >> the Pis are all using an outdated firmware. In their distro they put the >> firmware already under the alternates systems, but it just lacks the SAE >> offload support that is required to make WPA3 work. The linux-firmware >> version does the trick nicely. >> >> I documented what I did to make this work on Pi5 (note that I normally >> use Fedora on Pi4 and thus never encountered this issue) >> >> https://holtmann.dev/enabling-wpa3-on-raspberry-pi/ >> >> However you need to use iwd and not hope that you get a wpa_supplicant >> released version that will work. >> >> So whole game of wpa_supplicant is vendor specific to the company that >> provides the driver is also insane, but that is another story. Use iwd >> and you can most likely have WPA3 support if you have the right firmware. >> > > wpa_supplicant is perfectly fine if the necessary patches are > backported, as Fedora has done: > https://src.fedoraproject.org/rpms/wpa_supplicant/c/99f4bf2096d3976cee01c499d7a30c1376f5f0f7 my point exactly. Tell me when the last hostap release was and how much delta that has to HEAD. So the wpa_supplicant you have in Fedora is essentially yet another fork of an upstream project. One of many. Reality is there is limited interest to make WiFi great on Linux. And if you really look honestly, then you realize it is pretty bad. Regards Marcel
Arend Van Spriel <arend.vanspriel@broadcom.com> writes: > On December 17, 2023 12:25:23 PM Kalle Valo <kvalo@kernel.org> wrote: > >> Hector Martin <marcan@marcan.st> wrote: >> >>> Using the WSEC command instead of sae_password seems to be the supported >>> mechanism on newer firmware, and also how the brcmdhd driver does it. >>> >>> According to user reports [1], the sae_password codepath doesn't actually >>> work on machines with Cypress chips anyway, so no harm in removing it. >>> >>> This makes WPA3 work with iwd, or with wpa_supplicant pending a support >>> patchset [2]. >>> >>> [1] https://rachelbythebay.com/w/2023/11/06/wpa3/ >>> [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html >>> >>> Signed-off-by: Hector Martin <marcan@marcan.st> >>> Reviewed-by: Neal Gompa <neal@gompa.dev> >> >> Arend, what do you think? >> >> We recently talked about people testing brcmfmac patches, has anyone else >> tested this? > > Not sure I already replied so maybe I am repeating myself. I would > prefer to keep the Cypress sae_password path as well although it > reportedly does not work. The vendor support in the driver can be used > to accommodate for that. The other option would be to have people with > Cypress chipset test this patch. If that works for both we can > consider dropping the sae_password path. Ok, thanks for checking. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Tue, Nov 7, 2023 at 1:06 AM Hector Martin <marcan@marcan.st> wrote:
>
> Using the WSEC command instead of sae_password seems to be the supported
> mechanism on newer firmware, and also how the brcmdhd driver does it.
>
> According to user reports [1], the sae_password codepath doesn't actually
> work on machines with Cypress chips anyway, so no harm in removing it.
>
> This makes WPA3 work with iwd, or with wpa_supplicant pending a support
> patchset [2].
>
> [1] https://rachelbythebay.com/w/2023/11/06/wpa3/
> [2] http://lists.infradead.org/pipermail/hostap/2023-July/041653.html
>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 46 +++++++++-------------
> .../broadcom/brcm80211/brcmfmac/fwil_types.h | 2 +-
> 2 files changed, 20 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 2a90bb24ba77..138af70a33b8 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -1687,52 +1687,44 @@ static u16 brcmf_map_fw_linkdown_reason(const struct brcmf_event_msg *e)
> return reason;
> }
>
> -static int brcmf_set_pmk(struct brcmf_if *ifp, const u8 *pmk_data, u16 pmk_len)
> +static int brcmf_set_wsec(struct brcmf_if *ifp, const u8 *key, u16 key_len, u16 flags)
> {
> struct brcmf_pub *drvr = ifp->drvr;
> struct brcmf_wsec_pmk_le pmk;
> int err;
>
> + if (key_len > sizeof(pmk.key)) {
> + bphy_err(drvr, "key must be less than %zu bytes\n",
> + sizeof(pmk.key));
> + return -EINVAL;
> + }
> +
> memset(&pmk, 0, sizeof(pmk));
>
> - /* pass pmk directly */
> - pmk.key_len = cpu_to_le16(pmk_len);
> - pmk.flags = cpu_to_le16(0);
> - memcpy(pmk.key, pmk_data, pmk_len);
> + /* pass key material directly */
> + pmk.key_len = cpu_to_le16(key_len);
> + pmk.flags = cpu_to_le16(flags);
> + memcpy(pmk.key, key, key_len);
>
> - /* store psk in firmware */
> + /* store key material in firmware */
> err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SET_WSEC_PMK,
> &pmk, sizeof(pmk));
> if (err < 0)
> bphy_err(drvr, "failed to change PSK in firmware (len=%u)\n",
> - pmk_len);
> + key_len);
>
> return err;
> }
>
> +static int brcmf_set_pmk(struct brcmf_if *ifp, const u8 *pmk_data, u16 pmk_len)
> +{
> + return brcmf_set_wsec(ifp, pmk_data, pmk_len, 0);
> +}
> +
> static int brcmf_set_sae_password(struct brcmf_if *ifp, const u8 *pwd_data,
> u16 pwd_len)
> {
> - struct brcmf_pub *drvr = ifp->drvr;
> - struct brcmf_wsec_sae_pwd_le sae_pwd;
> - int err;
> -
> - if (pwd_len > BRCMF_WSEC_MAX_SAE_PASSWORD_LEN) {
> - bphy_err(drvr, "sae_password must be less than %d\n",
> - BRCMF_WSEC_MAX_SAE_PASSWORD_LEN);
> - return -EINVAL;
> - }
> -
> - sae_pwd.key_len = cpu_to_le16(pwd_len);
> - memcpy(sae_pwd.key, pwd_data, pwd_len);
> -
> - err = brcmf_fil_iovar_data_set(ifp, "sae_password", &sae_pwd,
> - sizeof(sae_pwd));
> - if (err < 0)
> - bphy_err(drvr, "failed to set SAE password in firmware (len=%u)\n",
> - pwd_len);
> -
> - return err;
> + return brcmf_set_wsec(ifp, pwd_data, pwd_len, BRCMF_WSEC_PASSPHRASE);
> }
>
> static void brcmf_link_down(struct brcmf_cfg80211_vif *vif, u16 reason,
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> index 611d1a6aabb9..b68c46caabe8 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> @@ -584,7 +584,7 @@ struct brcmf_wsec_key_le {
> struct brcmf_wsec_pmk_le {
> __le16 key_len;
> __le16 flags;
> - u8 key[2 * BRCMF_WSEC_MAX_PSK_LEN + 1];
> + u8 key[BRCMF_WSEC_MAX_SAE_PASSWORD_LEN];
> };
>
> /**
>
> ---
> base-commit: ffc253263a1375a65fa6c9f62a893e9767fbebfa
> change-id: 20231107-brcmfmac-wpa3-9e5f66e8be34
>
> Best regards,
> --
> Hector Martin <marcan@marcan.st>
>
>
Looks good to me.
Reviewed-by: Neal Gompa <neal@gompa.dev>
--
真実はいつも一つ！/ Always, there's only one truth!
© 2016 - 2025 Red Hat, Inc.