Re: [PATCH] wifi: brcmfmac: cfg80211: Use WSEC to set SAE password

[Arend Van Spriel <arend.vanspriel@xxxxxxxxxxxx>]

On December 22, 2023 6:28:12 AM Hector Martin <marcan@xxxxxxxxx> wrote:

> On 2023/12/21 23:04, Arend van Spriel wrote:
> > From: Hector Martin <marcan@xxxxxxxxx>
> >
> > 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@xxxxxxxxx>
> > Reviewed-by: Neal Gompa <neal@xxxxxxxxx>
> > Signed-off-by: Paweł Drewniak <czajernia@xxxxxxxxx>
> > Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxx>
> > [arend.vanspriel@xxxxxxxxxxxx: use multi-vendor framework]
> > Signed-off-by: Arend van Spriel <arend.vanspriel@xxxxxxxxxxxx>
> > ---
> > Here is how the multi-vendor code could support both Cypress and
> > WCC mobility chips. As said it might be easier to just override
> > entire cfg80211 callback operations.
> >
> > Regards,
> > Arend
> > ---
> > .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 13 +++++++---
> > .../broadcom/brcm80211/brcmfmac/cfg80211.h    |  3 +++
> > .../broadcom/brcm80211/brcmfmac/fwil.c        |  1 +
> > .../broadcom/brcm80211/brcmfmac/fwil_types.h  |  2 +-
> > .../broadcom/brcm80211/brcmfmac/fwvid.h       | 14 ++++++++++
> > .../broadcom/brcm80211/brcmfmac/wcc/core.c    | 26 +++++++++++++++++++
> > 6 files changed, 55 insertions(+), 4 deletions(-)
> [snip]
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/wcc/core.c 
> > b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/wcc/core.c
> > index 5573a47766ad..01025d5c137b 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/wcc/core.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/wcc/core.c
> > @@ -7,6 +7,7 @@
> > #include <core.h>
> > #include <bus.h>
> > #include <fwvid.h>
> > +#include <fwil.h>
> >
> > #include "vops.h"
> >
> > @@ -21,7 +22,32 @@ static void brcmf_wcc_detach(struct brcmf_pub *drvr)
> > pr_debug("%s: executing\n", __func__);
> > }
> >
> > +static int brcmf_wcc_set_sae_pwd(struct brcmf_if *ifp,
> > + struct cfg80211_crypto_settings *crypto)
> > +{
> > + struct brcmf_pub *drvr = ifp->drvr;
> > + struct brcmf_wsec_pmk_le pmk;
> > + int err;
> > +
> > + memset(&pmk, 0, sizeof(pmk));
> > +
> > + /* pass pmk directly */
> > + pmk.key_len = cpu_to_le16(crypto->sae_pwd_len);
> > + pmk.flags = cpu_to_le16(BRCMF_WSEC_PASSPHRASE);
> > + memcpy(pmk.key, crypto->sae_pwd, crypto->sae_pwd_len);
> > +
> > + /* store psk 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",
> > + crypto->sae_pwd_len);
> > +
> > + return err;
> > +}
> > +
> > const struct brcmf_fwvid_ops brcmf_wcc_ops = {
> > .attach = brcmf_wcc_attach,
> > .detach = brcmf_wcc_detach,
> > + .set_sae_password = brcmf_wcc_set_sae_pwd,
> > };
>
> If we're going to move this into per-vendor code, we should also move
> the Cypress codepath repectively. Is there a reason why we can't just
> rename and export brcmf_set_wsec (as in my original patch) instead of
> duplicating the code here? Fundamentally this code already exists in
> common code for WPA support, so why not reuse it for SAE for WCC?

Agree. Just whipped up a first draft and this came out. Maybe I will make 
it a series, because there's more groundwork to be done like exporting all 
fwil functions and probably inlining a few to limit the exports.


Regards,
Arend

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature