| Message ID | 1296a42e233d30603a082a265e345c521ff3f573.1523526306.git.peter.oh@bowerswilkins.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | mesh: enable DFS channels in mesh mode | expand |
Hi Peter, Hi Jouni, Hi Masashi, while testing I realized that in order to get mesh-mode work with only sae_password set, I needed to make changes as shown below. It's not a bug introduced by your code because you only refactored it. Should it be fixed before or after your series was applied? On Thu, Apr 12, 2018 at 02:48:59AM -0700, peter.oh@bowerswilkins.com wrote: > From: Peter Oh <peter.oh@bowerswilkins.com> > > RSN initialization can be used in different phases > if mesh initialization and mesh join don't happen > in sequence such as DFS CAC is done in between, > hence factor it out to help convering the case. > > Signed-off-by: Peter Oh <peter.oh@bowerswilkins.com> > --- > wpa_supplicant/mesh.c | 73 +++++++++++++++++++++++++++++++-------------------- > wpa_supplicant/mesh.h | 1 + > 2 files changed, 45 insertions(+), 29 deletions(-) > > diff --git a/wpa_supplicant/mesh.c b/wpa_supplicant/mesh.c > index f2f417dca..8e0d5ebff 100644 > --- a/wpa_supplicant/mesh.c > +++ b/wpa_supplicant/mesh.c > @@ -147,6 +147,48 @@ static void wpas_mesh_copy_groups(struct hostapd_data *bss, > groups_size); > } > > +int wpas_mesh_init_rsn(struct wpa_supplicant *wpa_s) > +{ > + struct hostapd_iface *ifmsh = wpa_s->ifmsh; > + struct mesh_conf *mconf = wpa_s->ifmsh->mconf; > + struct wpa_ssid *ssid = wpa_s->current_ssid; > + struct hostapd_data *bss = ifmsh->bss[0]; > + static int default_groups[] = { 19, 20, 21, 25, 26, -1 }; > + size_t len; > + > + if (mconf->security != MESH_CONF_SEC_NONE) { > + if (ssid->passphrase == NULL) { sae_password has to be handled here... > + wpa_printf(MSG_ERROR, > + "mesh: Passphrase for SAE not configured"); > + return -1; > + } > + > + bss->conf->wpa = ssid->proto; > + bss->conf->wpa_key_mgmt = ssid->key_mgmt; > + > + if (wpa_s->conf->sae_groups && > + wpa_s->conf->sae_groups[0] > 0) { > + wpas_mesh_copy_groups(bss, wpa_s); > + } else { > + bss->conf->sae_groups = > + os_memdup(default_groups, > + sizeof(default_groups)); > + if (!bss->conf->sae_groups) > + return -1; > + } > + > + len = os_strlen(ssid->passphrase); and here > + bss->conf->ssid.wpa_passphrase = > + dup_binstr(ssid->passphrase, len); as well. > + > + wpa_s->mesh_rsn = mesh_rsn_auth_init(wpa_s, mconf); > + if (!wpa_s->mesh_rsn) > + return -1; > + } > + > + return 0; > +} > + Hence, once your series is applied, I reckon we should add this on top: From 30c1693f42326d4f927e76120492bc9593b8f739 Mon Sep 17 00:00:00 2001 From: Daniel Golle <daniel@makrotopia.org> Date: Fri, 13 Apr 2018 00:42:10 +0200 Subject: [PATCH] mesh: properly handle sae_password The recently introduced sae_password parameter is only handled properly in wpa_supplicant/sme.c while wpa_supplicant/mesh.c assumed that ssid->passphrase exclusively holds the secret. Import the logic from sme.c to mesh.c to allow having only sae_password set which otherwise throws this error: AP-ENABLED mesh: Passphrase for SAE not configured Init RSN failed. Deinit mesh... wlan1: interface state ENABLED->DISABLED AP-DISABLED Segmentation fault Signed-off-by: Daniel Golle <daniel@makrotopia.org> --- wpa_supplicant/mesh.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/wpa_supplicant/mesh.c b/wpa_supplicant/mesh.c index 22dec4822..0bf87245d 100644 --- a/wpa_supplicant/mesh.c +++ b/wpa_supplicant/mesh.c @@ -154,10 +154,14 @@ int wpas_mesh_init_rsn(struct wpa_supplicant *wpa_s) struct wpa_ssid *ssid = wpa_s->current_ssid; struct hostapd_data *bss = ifmsh->bss[0]; static int default_groups[] = { 19, 20, 21, 25, 26, -1 }; + const char *password; size_t len; if (mconf->security != MESH_CONF_SEC_NONE) { - if (ssid->passphrase == NULL) { + password = ssid->sae_password; + if (!password) + password = ssid->passphrase; + if (!password) { wpa_printf(MSG_ERROR, "mesh: Passphrase for SAE not configured"); return -1; @@ -177,9 +181,9 @@ int wpas_mesh_init_rsn(struct wpa_supplicant *wpa_s) return -1; } - len = os_strlen(ssid->passphrase); + len = os_strlen(password); bss->conf->ssid.wpa_passphrase = - dup_binstr(ssid->passphrase, len); + dup_binstr(password, len); wpa_s->mesh_rsn = mesh_rsn_auth_init(wpa_s, mconf); if (!wpa_s->mesh_rsn)
Hi Daniel, I prefer you rebase your change after the series applied if you don't mind. But it could take time until it gets checked in since it's not a small change. Hence you may send your change separately and if your patch is merged before the series, then I'll rebase my patchset. Thanks, Peter On 04/12/2018 04:00 PM, Daniel Golle wrote: > Hi Peter, > Hi Jouni, > Hi Masashi, > > while testing I realized that in order to get mesh-mode work with only > sae_password set, I needed to make changes as shown below. It's not a > bug introduced by your code because you only refactored it. Should it > be fixed before or after your series was applied? > > On Thu, Apr 12, 2018 at 02:48:59AM -0700, peter.oh@bowerswilkins.com wrote: >> From: Peter Oh <peter.oh@bowerswilkins.com> >> >> RSN initialization can be used in different phases >> if mesh initialization and mesh join don't happen >> in sequence such as DFS CAC is done in between, >> hence factor it out to help convering the case. >> >> Signed-off-by: Peter Oh <peter.oh@bowerswilkins.com> >> --- >> wpa_supplicant/mesh.c | 73 +++++++++++++++++++++++++++++++-------------------- >> wpa_supplicant/mesh.h | 1 + >> 2 files changed, 45 insertions(+), 29 deletions(-) >> >> diff --git a/wpa_supplicant/mesh.c b/wpa_supplicant/mesh.c >> index f2f417dca..8e0d5ebff 100644 >> --- a/wpa_supplicant/mesh.c >> +++ b/wpa_supplicant/mesh.c >> @@ -147,6 +147,48 @@ static void wpas_mesh_copy_groups(struct hostapd_data *bss, >> groups_size); >> } >> >> +int wpas_mesh_init_rsn(struct wpa_supplicant *wpa_s) >> +{ >> + struct hostapd_iface *ifmsh = wpa_s->ifmsh; >> + struct mesh_conf *mconf = wpa_s->ifmsh->mconf; >> + struct wpa_ssid *ssid = wpa_s->current_ssid; >> + struct hostapd_data *bss = ifmsh->bss[0]; >> + static int default_groups[] = { 19, 20, 21, 25, 26, -1 }; >> + size_t len; >> + >> + if (mconf->security != MESH_CONF_SEC_NONE) { >> + if (ssid->passphrase == NULL) { > sae_password has to be handled here... > >> + wpa_printf(MSG_ERROR, >> + "mesh: Passphrase for SAE not configured"); >> + return -1; >> + } >> + >> + bss->conf->wpa = ssid->proto; >> + bss->conf->wpa_key_mgmt = ssid->key_mgmt; >> + >> + if (wpa_s->conf->sae_groups && >> + wpa_s->conf->sae_groups[0] > 0) { >> + wpas_mesh_copy_groups(bss, wpa_s); >> + } else { >> + bss->conf->sae_groups = >> + os_memdup(default_groups, >> + sizeof(default_groups)); >> + if (!bss->conf->sae_groups) >> + return -1; >> + } >> + >> + len = os_strlen(ssid->passphrase); > and here > >> + bss->conf->ssid.wpa_passphrase = >> + dup_binstr(ssid->passphrase, len); > as well. > > >> + >> + wpa_s->mesh_rsn = mesh_rsn_auth_init(wpa_s, mconf); >> + if (!wpa_s->mesh_rsn) >> + return -1; >> + } >> + >> + return 0; >> +} >> + > Hence, once your series is applied, I reckon we should add this on top: > > From 30c1693f42326d4f927e76120492bc9593b8f739 Mon Sep 17 00:00:00 2001 > From: Daniel Golle <daniel@makrotopia.org> > Date: Fri, 13 Apr 2018 00:42:10 +0200 > Subject: [PATCH] mesh: properly handle sae_password > > The recently introduced sae_password parameter is only handled properly > in wpa_supplicant/sme.c while wpa_supplicant/mesh.c assumed that > ssid->passphrase exclusively holds the secret. > Import the logic from sme.c to mesh.c to allow having only sae_password > set which otherwise throws this error: > AP-ENABLED > mesh: Passphrase for SAE not configured > Init RSN failed. Deinit mesh... > wlan1: interface state ENABLED->DISABLED > AP-DISABLED > Segmentation fault > > Signed-off-by: Daniel Golle <daniel@makrotopia.org> > --- > wpa_supplicant/mesh.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/wpa_supplicant/mesh.c b/wpa_supplicant/mesh.c > index 22dec4822..0bf87245d 100644 > --- a/wpa_supplicant/mesh.c > +++ b/wpa_supplicant/mesh.c > @@ -154,10 +154,14 @@ int wpas_mesh_init_rsn(struct wpa_supplicant *wpa_s) > struct wpa_ssid *ssid = wpa_s->current_ssid; > struct hostapd_data *bss = ifmsh->bss[0]; > static int default_groups[] = { 19, 20, 21, 25, 26, -1 }; > + const char *password; > size_t len; > > if (mconf->security != MESH_CONF_SEC_NONE) { > - if (ssid->passphrase == NULL) { > + password = ssid->sae_password; > + if (!password) > + password = ssid->passphrase; > + if (!password) { > wpa_printf(MSG_ERROR, > "mesh: Passphrase for SAE not configured"); > return -1; > @@ -177,9 +181,9 @@ int wpas_mesh_init_rsn(struct wpa_supplicant *wpa_s) > return -1; > } > > - len = os_strlen(ssid->passphrase); > + len = os_strlen(password); > bss->conf->ssid.wpa_passphrase = > - dup_binstr(ssid->passphrase, len); > + dup_binstr(password, len); > > wpa_s->mesh_rsn = mesh_rsn_auth_init(wpa_s, mconf); > if (!wpa_s->mesh_rsn)
On Thu, Apr 12, 2018 at 07:32:26PM -0700, Peter Oh wrote: > I prefer you rebase your change after the series applied if you don't mind. I do mind.. There is no point in introducing new segmentation faults for a known issue and it is clear that this mesh patchset needs additional work regardless taken into account the fixes that Masashi sent. > Hence you may send your change separately and if your patch is merged before > the series, then I'll rebase my patchset. I applied this patch from Daniel now to get sae_password working. Please rebase the mesh DFS set on top of the updated hostap.git and pull in the fixes from the two patches that Masashi sent to the appropriate commits in the set. Each new patch should apply cleanly and not introduce compiler warnings. > >From: Daniel Golle <daniel@makrotopia.org> > >Date: Fri, 13 Apr 2018 00:42:10 +0200 > >Subject: [PATCH] mesh: properly handle sae_password > > > >The recently introduced sae_password parameter is only handled properly > >in wpa_supplicant/sme.c while wpa_supplicant/mesh.c assumed that > >ssid->passphrase exclusively holds the secret. > >Import the logic from sme.c to mesh.c to allow having only sae_password > >set which otherwise throws this error: > >AP-ENABLED > >mesh: Passphrase for SAE not configured > >Init RSN failed. Deinit mesh... > >wlan1: interface state ENABLED->DISABLED > >AP-DISABLED > >Segmentation fault This segmentation fault part was introduced in these new mesh DFS patches. Without them (i.e., the hostap.git master branch before today), I saw no segmentation fault here -- the setup just failed cleanly with that "passphrase for SAE not configured" message.
On 04/13/2018 02:39 AM, Jouni Malinen wrote: > On Thu, Apr 12, 2018 at 07:32:26PM -0700, Peter Oh wrote: >> I prefer you rebase your change after the series applied if you don't mind. > I do mind.. There is no point in introducing new segmentation faults for > a known issue and it is clear that this mesh patchset needs additional > work regardless taken into account the fixes that Masashi sent. > >> Hence you may send your change separately and if your patch is merged before >> the series, then I'll rebase my patchset. > I applied this patch from Daniel now to get sae_password working. Please > rebase the mesh DFS set on top of the updated hostap.git and pull in the > fixes from the two patches that Masashi sent to the appropriate commits > in the set. Each new patch should apply cleanly and not introduce > compiler warnings. I'll do rebase. >>> From: Daniel Golle <daniel@makrotopia.org> >>> Date: Fri, 13 Apr 2018 00:42:10 +0200 >>> Subject: [PATCH] mesh: properly handle sae_password >>> >>> The recently introduced sae_password parameter is only handled properly >>> in wpa_supplicant/sme.c while wpa_supplicant/mesh.c assumed that >>> ssid->passphrase exclusively holds the secret. >>> Import the logic from sme.c to mesh.c to allow having only sae_password >>> set which otherwise throws this error: >>> AP-ENABLED >>> mesh: Passphrase for SAE not configured >>> Init RSN failed. Deinit mesh... >>> wlan1: interface state ENABLED->DISABLED >>> AP-DISABLED >>> Segmentation fault > This segmentation fault part was introduced in these new mesh DFS > patches. Without them (i.e., the hostap.git master branch before today), > I saw no segmentation fault here -- the setup just failed cleanly with > that "passphrase for SAE not configured" message. What is the compiling option generating segmentation fault? I want to try. Thanks, Peter
Hi Peter, On Fri, Apr 13, 2018 at 11:54:29AM -0700, Peter Oh wrote: > > > > AP-ENABLED > > > > mesh: Passphrase for SAE not configured > > > > Init RSN failed. Deinit mesh... > > > > wlan1: interface state ENABLED->DISABLED > > > > AP-DISABLED > > > > Segmentation fault > > This segmentation fault part was introduced in these new mesh DFS > > patches. Without them (i.e., the hostap.git master branch before today), > > I saw no segmentation fault here -- the setup just failed cleanly with > > that "passphrase for SAE not configured" message. > What is the compiling option generating segmentation fault? I want to try. This was on Atheros AR7240 SoC running OpenWrt (ie. MIPS32Kc bytecode). The easiest way to reproduce this would be to grab an arbitary ath9k-based 802.11a router and build a firmware using: git clone https://git.openwrt.org/openwrt/openwrt.git/ cd openwrt # remove the fix rm package/network/services/hostapd/patches/020-mesh-properly-handle-sae_password.patch make menuconfig # select your router model make # add -j4 or such to have build in parallel Once you flashed the firmware, configure a wireless network using UCI on the target: uci set wireless.radio1.channel='100' uci set wireless.radio1.country='DE' uci set wireless.default_radio1.mode=mesh uci set wireless.default_radio1.encryption=psk2 uci set wireless.default_radio1.key='freifunk' uci commit wireless wifi And you'll see wpa_supplicant segfaulting once CAC is done. Cheers Daniel
On 04/13/2018 12:48 PM, Daniel Golle wrote:
> And you'll see wpa_supplicant segfaulting once CAC is done.
So we won't see if no DFS channel is used in mesh. right?
And you patch fixes the error and segmentation fault on both of non-DFS
and DFS channels case?
If so, what do you expect with the steps of applying patches below?
apply your sae_password patch on current hostap.git -> apply my mesh DFS
patchset.
Will it solve the both sae_password and segmentation fault?
BTW, according to your explanation, you see the segmentation fault even
without my mesh DFS patchset, am I correct?
Thanks,
Peter
diff --git a/wpa_supplicant/mesh.c b/wpa_supplicant/mesh.c index f2f417dca..8e0d5ebff 100644 --- a/wpa_supplicant/mesh.c +++ b/wpa_supplicant/mesh.c @@ -147,6 +147,48 @@ static void wpas_mesh_copy_groups(struct hostapd_data *bss, groups_size); } +int wpas_mesh_init_rsn(struct wpa_supplicant *wpa_s) +{ + struct hostapd_iface *ifmsh = wpa_s->ifmsh; + struct mesh_conf *mconf = wpa_s->ifmsh->mconf; + struct wpa_ssid *ssid = wpa_s->current_ssid; + struct hostapd_data *bss = ifmsh->bss[0]; + static int default_groups[] = { 19, 20, 21, 25, 26, -1 }; + size_t len; + + if (mconf->security != MESH_CONF_SEC_NONE) { + if (ssid->passphrase == NULL) { + wpa_printf(MSG_ERROR, + "mesh: Passphrase for SAE not configured"); + return -1; + } + + bss->conf->wpa = ssid->proto; + bss->conf->wpa_key_mgmt = ssid->key_mgmt; + + if (wpa_s->conf->sae_groups && + wpa_s->conf->sae_groups[0] > 0) { + wpas_mesh_copy_groups(bss, wpa_s); + } else { + bss->conf->sae_groups = + os_memdup(default_groups, + sizeof(default_groups)); + if (!bss->conf->sae_groups) + return -1; + } + + len = os_strlen(ssid->passphrase); + bss->conf->ssid.wpa_passphrase = + dup_binstr(ssid->passphrase, len); + + wpa_s->mesh_rsn = mesh_rsn_auth_init(wpa_s, mconf); + if (!wpa_s->mesh_rsn) + return -1; + } + + return 0; +} + static int wpa_supplicant_mesh_init(struct wpa_supplicant *wpa_s, struct wpa_ssid *ssid, @@ -291,35 +333,8 @@ static int wpa_supplicant_mesh_init(struct wpa_supplicant *wpa_s, return -1; } - if (mconf->security != MESH_CONF_SEC_NONE) { - if (ssid->passphrase == NULL) { - wpa_printf(MSG_ERROR, - "mesh: Passphrase for SAE not configured"); - goto out_free; - } - - bss->conf->wpa = ssid->proto; - bss->conf->wpa_key_mgmt = ssid->key_mgmt; - - if (wpa_s->conf->sae_groups && - wpa_s->conf->sae_groups[0] > 0) { - wpas_mesh_copy_groups(bss, wpa_s); - } else { - bss->conf->sae_groups = - os_memdup(default_groups, - sizeof(default_groups)); - if (!bss->conf->sae_groups) - goto out_free; - } - - len = os_strlen(ssid->passphrase); - bss->conf->ssid.wpa_passphrase = - dup_binstr(ssid->passphrase, len); - - wpa_s->mesh_rsn = mesh_rsn_auth_init(wpa_s, mconf); - if (!wpa_s->mesh_rsn) - goto out_free; - } + if (wpas_mesh_init_rsn(wpa_s)) + goto out_free; wpa_supplicant_conf_ap_ht(wpa_s, ssid, conf); diff --git a/wpa_supplicant/mesh.h b/wpa_supplicant/mesh.h index 2e2f3cfbf..995210236 100644 --- a/wpa_supplicant/mesh.h +++ b/wpa_supplicant/mesh.h @@ -22,6 +22,7 @@ int wpas_mesh_peer_remove(struct wpa_supplicant *wpa_s, const u8 *addr); int wpas_mesh_peer_add(struct wpa_supplicant *wpa_s, const u8 *addr, int duration); void wpas_join_mesh(struct wpa_supplicant *wpa_s); +int wpas_mesh_init_rsn(struct wpa_supplicant *wpa_s); #ifdef CONFIG_MESH