On Thu, Oct 3, 2019 at 5:53 PM Brian Norris <briannorris@xxxxxxxxxxxx> wrote:
> On Mon, Aug 5, 2019 at 3:40 PM Eric Caruso <ejcaruso@xxxxxxxxxxxx> wrote:
> > > > @@ -169,7 +195,9 @@ static void wpas_trigger_scan_cb(struct wpa_radio_work *work, int deinit)
> > > > return;
> > > > }
> > > >
> > > > - if (wpas_update_random_addr_disassoc(wpa_s) < 0) {
> > > > + if (wpa_s->mac_addr_rand_enable & MAC_ADDR_RAND_SCAN)
> > > > + wpa_setup_mac_addr_rand_params(params, wpa_s->mac_addr_scan);
> > > > + else if (wpas_update_random_addr_disassoc(wpa_s) < 0) {
> > >
> > > What is this trying to do and why? The commit message seems to imply
> > > that this should not have any changes in behavior, but this looks like a
> > > potential change.
...
> I think this hunk needs to die.
Local testing shows that killing it is not sufficient, in local
testing. I haven't quite figured out exactly why though...
...but we definitely should at least be checking for "wpa_s->wpa_state
< WPA_AUTHENTICATING", at least.
> > Both of these are employing MAC address randomization in different
> > ways. I can make it so we try both here, but as far as I can tell,
>
> I'm not really sure what you mean by "both of these". But in case this
> is what you're talking about: wpas_update_random_addr_disassoc() isn't
> actually related to MAC address randomization for scanning AFAICT.
And I was wrong here: wpas_update_random_addr_disassoc() does deploy a
similar feature, based off the 'preassoc_mac_addr' config setting. So
I guess that's what you meant by "both of these."
> > since this implementation asks the driver to randomize the address
> > per-probe then if it is active it will take priority over the existing
>
> The 'mac_addr_rand_enable' feature flag is not per-probe -- it's a
> sticky setting in wpa_s, and it only means that *if we're not
> associated*, we should use a random address in probes. And the
> parameter setup is done properly earlier, in wpa_supplicant_scan() --
> I don't think you need to repeat (and override) it here.
>
> > implementation anyway. Now that you point this out, though, we could
> > set this bit even if the driver or module doesn't actually support the
> > hardware random MAC implementation, so it's probably worth doing both
> > here instead.
Yes, the "else if" seems a little off -- we should probably give the
option of doing both.
Brian
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
