Hi,
Apologies for the long CC list, it seems lots of people were either
involved with the scanning code or could know how it should work :)
After having two bug reports about the BSS list/probe_resp variable in
mac80211 and a very detailed analysis of the problem from Bill Moss, I
think I've understood the problem now but I'm not sure how to proceed
because I do not understand the reason for "Do not allow beacons to
override data from probe responses".
I'll get into more detail of that code in a bit, but let me give a
global overview first for those not intimately familiar with the code.
In the MLME/scan code, mac80211 keeps a list of BSS info structures (for
each hardware that is registered to the system), each of the structures
containing information on the BSS, see struct ieee80211_sta_bss in
ieee80211_i.h.
When we scan, we will always try to keep each of the BSS structs we have
updated with the last information received, but there is code to prevent
updating it from a beacon when we have previously received probe
responses.
Now, this code has two mostly orthogonal problems.
The first one is that there is no actual expiration of BSS structs. Each
BSS struct has a 'last_update' member that contains (in jiffies) the
time this item was last updated. This means that we accumulate BSS
information forever, but due to the 'last_update' only the last few
items will be returned to the user on asking for a scan result. This
obviously has problems since a rogue station could bombard us with fake
probe responses and cause us to build a huge BSS list which is never
again freed until the hardware is deregistered. This will need to be
fixed, of course.
The second problem is the one Bill Moss and Vladimir Koutny were running
into. As soon as we receive a probe response from a BSS, we never again
update it from beacons, hence userspace can never find an AP during
passive scanning if we've seen that BSSID before during an active scan.
In Vladimir Koutny's case this is due to a hardware bug (though I can't
see why any hardware would behave that way), in Bill Moss's case I'm not
entirely sure why he's not getting probe responses, maybe the AP is just
not quick enough.
In any case, the problem then is that last_update is updated only along
with the remaining information in a BSS struct, which is quite correct.
The question, however, is: why are beacons not allowed to override probe
response information? That is, why does this piece of code exist:
if (sdata->vif.type != IEEE80211_IF_TYPE_IBSS &&
bss->probe_resp && beacon) {
/* STA mode:
* Do not allow beacon to override data from Probe Response. */
ieee80211_rx_bss_put(dev, bss);
return;
}
I failed to find any explanation for it in the git archive (it was part
of the first code drop from Jiri that made it into a wireless tree and I
don't have any older git archives handy) nor in my email though I
vaguely remember there was a reason for it (or I may have made it up).
Bill Moss reports that removing this piece of code (and then the
probe_resp variable completely since it'll be write-only) fixes the
problem, and it's obvious why.
The only thing I'm not sure about is whether that will introduce
regressions because probe responses can contain better information than
beacons. The only reason I can find for this would be an AP that has
multiple SSIDs with different security settings on each and announces
those only in the probe responses (and otherwise uses a hidden SSID),
but we only handle that now, much later than the code there that was
already present.
However, that actually seems to have another bug, if you have such an AP
will you find two BSSes, one with a hidden and one with a visible SSID?
Any hints about this code or the desired behaviour would be greatly
appreciated.
Thanks,
johannes
On Sun, Mar 30, 2008 at 10:33:19AM +0200, Johannes Berg wrote:
> After having two bug reports about the BSS list/probe_resp variable i=
n
> mac80211 and a very detailed analysis of the problem from Bill Moss, =
I
> think I've understood the problem now but I'm not sure how to proceed
> because I do not understand the reason for "Do not allow beacons to
> override data from probe responses".
The main (and only) reason for this is support for multi-SSID
configurations (hidden SSID and/or multiple SSIDs with the same BSSID,
but with different security parameters). Only the Probe Response frames
are guaranteed to include correct information for <BSSID,SSID> pair in
these cases and because of that, Beacon frames are not allowed to updat=
e
things like SSID or WPA/RSN IE.
> The question, however, is: why are beacons not allowed to override pr=
obe
> response information? That is, why does this piece of code exist:
>=20
> if (sdata->vif.type !=3D IEEE80211_IF_TYPE_IBSS &&
> bss->probe_resp && beacon) {
> /* STA mode:
> * Do not allow beacon to override data from Probe Re=
sponse. */
> ieee80211_rx_bss_put(dev, bss);
> return;
> } =20
This controls which BSS info fields are updated based on both Beacon an=
d
Probe Response frames and which are updated only based on Probe Respons=
e
frames.
One example of information that should not be overridden by Beacon
frames is WPA and RSN IE since these could differ based on the SSID in
multi-SSID configurations. An example of information that must be
updated from every Beacon frame would be ERP IE. Finally, there are als=
o
more complex cases, like the Capability Information, which should
probably be handled separately for each subfield (e.g., Privacy could b=
e
per-SSID and Short Slot Time for every SSID of the BSSID).
> Bill Moss reports that removing this piece of code (and then the
> probe_resp variable =EF=BB=BFcompletely since it'll be write-only) fi=
xes the
> problem, and it's obvious why.
I don't think that this would be the correct fix for this issue.
> The only thing I'm not sure about is whether that will introduce
> regressions because probe responses can contain better information th=
an
> beacons. The only reason I can find for this would be an AP that has
> multiple SSIDs with different security settings on each and announces
> those only in the probe responses (and otherwise uses a hidden SSID),
> but we only handle that now, much later than the code there that was
> already present.
I'm not sure whether the current code is correct for all information
from Beacon/ProbeResp (i.e., some parts might need to be moved to be
before/after this check), but changes to WPA/RSN IE are after this
return-on-Beacon for a reason and removing that check would break some
multi-SSID networks.
> However, that actually seems to have another bug, if you have such an=
AP
> will you find two BSSes, one with a hidden and one with a visible SSI=
D?
That is by design.
> Any hints about this code or the desired behaviour would be greatly
> appreciated.
I think that we will need to evaluate whether the current implementatio=
n
is updating all pieces of information in correct place and still
maintain possibility of not using Beacon frames to update some fields.
There might be cases where the old information from ProbeResp should
expire (though, if we are still associated with that BSSID,SSID pair, w=
e
should use Probe Request to update this information instead of just
blindly throughing it away).
Any information that has to be same for all SSIDs in multi-SSID (single
BSSID) configuration can be updated based on Beacon frames since the
SSIDs for a specific SSID would not contain any different information
for these parts. However, fields that can be configured per-SSID, must
be handled separately in order to be able to support multi-SSID
configuration with different security policies per SSID. This would
mainly included Privacy flag in the capability field and security
related IEs (WPA IE, RSN IE), but there may be something else that woul=
d
benefit of similar handling.
--=20
Jouni Malinen PGP id EFC895F=
A
T24gU3VuLCBNYXIgMzAsIDIwMDggYXQgMTE6MzMgQU0sIEpvaGFubmVzIEJlcmcKPGpvaGFubmVz
QHNpcHNvbHV0aW9ucy5uZXQ+IHdyb3RlOgo+IEhpLAo+Cj4gIEFwb2xvZ2llcyBmb3IgdGhlIGxv
bmcgQ0MgbGlzdCwgaXQgc2VlbXMgbG90cyBvZiBwZW9wbGUgd2VyZSBlaXRoZXIKPiAgaW52b2x2
ZWQgd2l0aCB0aGUgc2Nhbm5pbmcgY29kZSBvciBjb3VsZCBrbm93IGhvdyBpdCBzaG91bGQgd29y
ayA6KQo+Cj4gIEFmdGVyIGhhdmluZyB0d28gYnVnIHJlcG9ydHMgYWJvdXQgdGhlIEJTUyBsaXN0
L3Byb2JlX3Jlc3AgdmFyaWFibGUgaW4KPiAgbWFjODAyMTEgYW5kIGEgdmVyeSBkZXRhaWxlZCBh
bmFseXNpcyBvZiB0aGUgcHJvYmxlbSBmcm9tIEJpbGwgTW9zcywgSQo+ICB0aGluayBJJ3ZlIHVu
ZGVyc3Rvb2QgdGhlIHByb2JsZW0gbm93IGJ1dCBJJ20gbm90IHN1cmUgaG93IHRvIHByb2NlZWQK
PiAgYmVjYXVzZSBJIGRvIG5vdCB1bmRlcnN0YW5kIHRoZSByZWFzb24gZm9yICJEbyBub3QgYWxs
b3cgYmVhY29ucyB0bwo+ICBvdmVycmlkZSBkYXRhIGZyb20gcHJvYmUgcmVzcG9uc2VzIi4KPgo+
ICBJJ2xsIGdldCBpbnRvIG1vcmUgZGV0YWlsIG9mIHRoYXQgY29kZSBpbiBhIGJpdCwgYnV0IGxl
dCBtZSBnaXZlIGEKPiAgZ2xvYmFsIG92ZXJ2aWV3IGZpcnN0IGZvciB0aG9zZSBub3QgaW50aW1h
dGVseSBmYW1pbGlhciB3aXRoIHRoZSBjb2RlLgo+Cj4gIEluIHRoZSBNTE1FL3NjYW4gY29kZSwg
bWFjODAyMTEga2VlcHMgYSBsaXN0IG9mIEJTUyBpbmZvIHN0cnVjdHVyZXMgKGZvcgo+ICBlYWNo
IGhhcmR3YXJlIHRoYXQgaXMgcmVnaXN0ZXJlZCB0byB0aGUgc3lzdGVtKSwgZWFjaCBvZiB0aGUg
c3RydWN0dXJlcwo+ICBjb250YWluaW5nIGluZm9ybWF0aW9uIG9uIHRoZSBCU1MsIHNlZSBzdHJ1
Y3QgaWVlZTgwMjExX3N0YV9ic3MgaW4KPiAgaWVlZTgwMjExX2kuaC4KPgo+ICBXaGVuIHdlIHNj
YW4sIHdlIHdpbGwgYWx3YXlzIHRyeSB0byBrZWVwIGVhY2ggb2YgdGhlIEJTUyBzdHJ1Y3RzIHdl
IGhhdmUKPiAgdXBkYXRlZCB3aXRoIHRoZSBsYXN0IGluZm9ybWF0aW9uIHJlY2VpdmVkLCBidXQg
dGhlcmUgaXMgY29kZSB0byBwcmV2ZW50Cj4gIHVwZGF0aW5nIGl0IGZyb20gYSBiZWFjb24gd2hl
biB3ZSBoYXZlIHByZXZpb3VzbHkgcmVjZWl2ZWQgcHJvYmUKPiAgcmVzcG9uc2VzLgo+Cj4gIE5v
dywgdGhpcyBjb2RlIGhhcyB0d28gbW9zdGx5IG9ydGhvZ29uYWwgcHJvYmxlbXMuCj4KPiAgVGhl
IGZpcnN0IG9uZSBpcyB0aGF0IHRoZXJlIGlzIG5vIGFjdHVhbCBleHBpcmF0aW9uIG9mIEJTUyBz
dHJ1Y3RzLiBFYWNoCj4gIEJTUyBzdHJ1Y3QgaGFzIGEgJ2xhc3RfdXBkYXRlJyBtZW1iZXIgdGhh
dCBjb250YWlucyAoaW4gamlmZmllcykgdGhlCj4gIHRpbWUgdGhpcyBpdGVtIHdhcyBsYXN0IHVw
ZGF0ZWQuIFRoaXMgbWVhbnMgdGhhdCB3ZSBhY2N1bXVsYXRlIEJTUwo+ICBpbmZvcm1hdGlvbiBm
b3JldmVyLCBidXQgZHVlIHRvIHRoZSAnbGFzdF91cGRhdGUnIG9ubHkgdGhlIGxhc3QgZmV3Cj4g
IGl0ZW1zIHdpbGwgYmUgcmV0dXJuZWQgdG8gdGhlIHVzZXIgb24gYXNraW5nIGZvciBhIHNjYW4g
cmVzdWx0LiBUaGlzCj4gIG9idmlvdXNseSBoYXMgcHJvYmxlbXMgc2luY2UgYSByb2d1ZSBzdGF0
aW9uIGNvdWxkIGJvbWJhcmQgdXMgd2l0aCBmYWtlCj4gIHByb2JlIHJlc3BvbnNlcyBhbmQgY2F1
c2UgdXMgdG8gYnVpbGQgYSBodWdlIEJTUyBsaXN0IHdoaWNoIGlzIG5ldmVyCj4gIGFnYWluIGZy
ZWVkIHVudGlsIHRoZSBoYXJkd2FyZSBpcyBkZXJlZ2lzdGVyZWQuIFRoaXMgd2lsbCBuZWVkIHRv
IGJlCj4gIGZpeGVkLCBvZiBjb3Vyc2UuCj4KPiAgVGhlIHNlY29uZCBwcm9ibGVtIGlzIHRoZSBv
bmUgQmlsbCBNb3NzIGFuZCBWbGFkaW1pciBLb3V0bnkgd2VyZSBydW5uaW5nCj4gIGludG8uIEFz
IHNvb24gYXMgd2UgcmVjZWl2ZSBhIHByb2JlIHJlc3BvbnNlIGZyb20gYSBCU1MsIHdlIG5ldmVy
IGFnYWluCj4gIHVwZGF0ZSBpdCBmcm9tIGJlYWNvbnMsIGhlbmNlIHVzZXJzcGFjZSBjYW4gbmV2
ZXIgZmluZCBhbiBBUCBkdXJpbmcKPiAgcGFzc2l2ZSBzY2FubmluZyBpZiB3ZSd2ZSBzZWVuIHRo
YXQgQlNTSUQgYmVmb3JlIGR1cmluZyBhbiBhY3RpdmUgc2Nhbi4KPiAgSW4gVmxhZGltaXIgS291
dG55J3MgY2FzZSB0aGlzIGlzIGR1ZSB0byBhIGhhcmR3YXJlIGJ1ZyAodGhvdWdoIEkgY2FuJ3QK
PiAgc2VlIHdoeSBhbnkgaGFyZHdhcmUgd291bGQgYmVoYXZlIHRoYXQgd2F5KSwgaW4gQmlsbCBN
b3NzJ3MgY2FzZSBJJ20gbm90Cj4gIGVudGlyZWx5IHN1cmUgd2h5IGhlJ3Mgbm90IGdldHRpbmcg
cHJvYmUgcmVzcG9uc2VzLCBtYXliZSB0aGUgQVAgaXMganVzdAo+ICBub3QgcXVpY2sgZW5vdWdo
Lgo+ICBJbiBhbnkgY2FzZSwgdGhlIHByb2JsZW0gdGhlbiBpcyB0aGF0IGxhc3RfdXBkYXRlIGlz
IHVwZGF0ZWQgb25seSBhbG9uZwo+ICB3aXRoIHRoZSByZW1haW5pbmcgaW5mb3JtYXRpb24gaW4g
YSBCU1Mgc3RydWN0LCB3aGljaCBpcyBxdWl0ZSBjb3JyZWN0Lgo+ICBUaGUgcXVlc3Rpb24sIGhv
d2V2ZXIsIGlzOiB3aHkgYXJlIGJlYWNvbnMgbm90IGFsbG93ZWQgdG8gb3ZlcnJpZGUgcHJvYmUK
PiAgcmVzcG9uc2UgaW5mb3JtYXRpb24/IFRoYXQgaXMsIHdoeSBkb2VzIHRoaXMgcGllY2Ugb2Yg
Y29kZSBleGlzdDoKPgo+ICAgICAgICAgaWYgKHNkYXRhLT52aWYudHlwZSAhPSBJRUVFODAyMTFf
SUZfVFlQRV9JQlNTICYmCj4gICAgICAgICAgICAgYnNzLT5wcm9iZV9yZXNwICYmIGJlYWNvbikg
ewo+ICAgICAgICAgICAgICAgICAvKiBTVEEgbW9kZToKPiAgICAgICAgICAgICAgICAgICogRG8g
bm90IGFsbG93IGJlYWNvbiB0byBvdmVycmlkZSBkYXRhIGZyb20gUHJvYmUgUmVzcG9uc2UuICov
Cj4gICAgICAgICAgICAgICAgIGllZWU4MDIxMV9yeF9ic3NfcHV0KGRldiwgYnNzKTsKPiAgICAg
ICAgICAgICAgICAgcmV0dXJuOwo+ICAgICAgICAgfQo+Cj4gIEkgZmFpbGVkIHRvIGZpbmQgYW55
IGV4cGxhbmF0aW9uIGZvciBpdCBpbiB0aGUgZ2l0IGFyY2hpdmUgKGl0IHdhcyBwYXJ0Cj4gIG9m
IHRoZSBmaXJzdCBjb2RlIGRyb3AgZnJvbSBKaXJpIHRoYXQgbWFkZSBpdCBpbnRvIGEgd2lyZWxl
c3MgdHJlZSBhbmQgSQo+ICBkb24ndCBoYXZlIGFueSBvbGRlciBnaXQgYXJjaGl2ZXMgaGFuZHkp
IG5vciBpbiBteSBlbWFpbCB0aG91Z2ggSQo+ICB2YWd1ZWx5IHJlbWVtYmVyIHRoZXJlIHdhcyBh
IHJlYXNvbiBmb3IgaXQgKG9yIEkgbWF5IGhhdmUgbWFkZSBpdCB1cCkuCj4KPiAgQmlsbCBNb3Nz
IHJlcG9ydHMgdGhhdCByZW1vdmluZyB0aGlzIHBpZWNlIG9mIGNvZGUgKGFuZCB0aGVuIHRoZQo+
ICBwcm9iZV9yZXNwIHZhcmlhYmxlIO+7v2NvbXBsZXRlbHkgc2luY2UgaXQnbGwgYmUgd3JpdGUt
b25seSkgZml4ZXMgdGhlCj4gIHByb2JsZW0sIGFuZCBpdCdzIG9idmlvdXMgd2h5Lgo+Cj4gIFRo
ZSBvbmx5IHRoaW5nIEknbSBub3Qgc3VyZSBhYm91dCBpcyB3aGV0aGVyIHRoYXQgd2lsbCBpbnRy
b2R1Y2UKPiAgcmVncmVzc2lvbnMgYmVjYXVzZSBwcm9iZSByZXNwb25zZXMgY2FuIGNvbnRhaW4g
YmV0dGVyIGluZm9ybWF0aW9uIHRoYW4KPiAgYmVhY29ucy4gVGhlIG9ubHkgcmVhc29uIEkgY2Fu
IGZpbmQgZm9yIHRoaXMgd291bGQgYmUgYW4gQVAgdGhhdCBoYXMKPiAgbXVsdGlwbGUgU1NJRHMg
d2l0aCBkaWZmZXJlbnQgc2VjdXJpdHkgc2V0dGluZ3Mgb24gZWFjaCBhbmQgYW5ub3VuY2VzCj4g
IHRob3NlIG9ubHkgaW4gdGhlIHByb2JlIHJlc3BvbnNlcyAoYW5kIG90aGVyd2lzZSB1c2VzIGEg
aGlkZGVuIFNTSUQpLAo+ICBidXQgd2Ugb25seSBoYW5kbGUgdGhhdCBub3csIG11Y2ggbGF0ZXIg
dGhhbiB0aGUgY29kZSB0aGVyZSB0aGF0IHdhcwo+ICBhbHJlYWR5IHByZXNlbnQuCj4KPiAgSG93
ZXZlciwgdGhhdCBhY3R1YWxseSBzZWVtcyB0byBoYXZlIGFub3RoZXIgYnVnLCBpZiB5b3UgaGF2
ZSBzdWNoIGFuIEFQCj4gIHdpbGwgeW91IGZpbmQgdHdvIEJTU2VzLCBvbmUgd2l0aCBhIGhpZGRl
biBhbmQgb25lIHdpdGggYSB2aXNpYmxlIFNTSUQ/Cj4KPiAgQW55IGhpbnRzIGFib3V0IHRoaXMg
Y29kZSBvciB0aGUgZGVzaXJlZCBiZWhhdmlvdXIgd291bGQgYmUgZ3JlYXRseQo+ICBhcHByZWNp
YXRlZC4KCkZvciBleGFtcGxlIFdNTSBwYXJhbWV0ZXJzIGFyZSBub3QgcHJlc2VudCBpbiBiZWFj
b24gYnV0IGFyZSBwcmVzZW50CmluIHByb2JlIHJlc3BvbnNlLgpXaGF0IGlzIG5lZWRlZCBoZXJl
IGlzIGJlaW5nIHNlbGVjdGl2ZSB3aGF0IGlzIHVwZGF0ZWQgYW5kIHdoYXQgbm90LgpXaGF0IGlz
IGZvciBzdXJlIHlvdSBuZWVkIGF0IGxlYXN0IG9uZSBwcm9iZSByZXNwb25zZSBiZWZvcmUgeW91
IGNhbiBhc3NvY2lhdGUuCgpUb21hcwoKPgo+ICBUaGFua3MsCj4gIGpvaGFubmVzCj4K
On Sun, 30 Mar 2008 10:33:19 +0200, Johannes Berg wrote:
> The question, however, is: why are beacons not allowed to override probe
> response information? That is, why does this piece of code exist:
>
> if (sdata->vif.type != IEEE80211_IF_TYPE_IBSS &&
> bss->probe_resp && beacon) {
> /* STA mode:
> * Do not allow beacon to override data from Probe Response. */
> ieee80211_rx_bss_put(dev, bss);
> return;
> }
>
> I failed to find any explanation for it in the git archive (it was part
> of the first code drop from Jiri that made it into a wireless tree and I
> don't have any older git archives handy) nor in my email though I
> vaguely remember there was a reason for it (or I may have made it up).
It was already in the initial Devicescape release:
1478 if (bss->probe_resp && beacon) {
1479 /* Do not allow beacon to override data from Probe Response. */
1480 ieee80211_rx_bss_put(dev, bss);
1481 return;
1482 }
The only reason I can think of is handling of hidden SSIDs. Seems it
was necessary at that time. I'd vote for removing it.
> However, that actually seems to have another bug, if you have such an AP
> will you find two BSSes, one with a hidden and one with a visible SSID?
That's what I would consider a correct behaviour. You in fact do have
two distinct BSSes. But I don't know how the current user space handles
that.
Thanks,
Jiri
--
Jiri Benc
SUSE Labs
On Sun, Mar 30, 2008 at 10:33:19AM +0200, Johannes Berg wrote:
> The first one is that there is no actual expiration of BSS structs. Each
> BSS struct has a 'last_update' member that contains (in jiffies) the
> time this item was last updated. This means that we accumulate BSS
> information forever, but due to the 'last_update' only the last few
> items will be returned to the user on asking for a scan result. This
> obviously has problems since a rogue station could bombard us with fake
> probe responses and cause us to build a huge BSS list which is never
> again freed until the hardware is deregistered. This will need to be
> fixed, of course.
Assuming this was fixed, does the rest of this issue go away?
It seems like it would.
> In any case, the problem then is that last_update is updated only along
> with the remaining information in a BSS struct, which is quite correct.
> The question, however, is: why are beacons not allowed to override probe
> response information? That is, why does this piece of code exist:
>
> if (sdata->vif.type != IEEE80211_IF_TYPE_IBSS &&
> bss->probe_resp && beacon) {
> /* STA mode:
> * Do not allow beacon to override data from Probe Response. */
> ieee80211_rx_bss_put(dev, bss);
> return;
> }
Should we consider allowing the beacon to update the info if
last_update is old enough to keep the BSS out of the scan results?
John
--
John W. Linville
[email protected]
On Tue, Apr 01, 2008 at 03:16:13PM +0200, Johannes Berg wrote:
> Right. But in that case, a beacon frame we receive will update the
> <BSSID,""> BSS structure, not the <BSSID,actual SSID> structure because
> the update code will find only the former since the SSID is hidden.
> TheBSS structure w/o SSID won't actually be used.
The Beacon frames would probably need to update all <BSSID,*> entries
for the fields that we decide need to be updated based on Beacons, not
just the <BSSID,"">... Alternatively, we could change the BSS table to
have just one entry for each BSSID and a list of SSID entries for each
BSSID (with SSID, WPA/RSN IE, Privacy flag, etc. from ProbeResp). This
would be internal to kernel, though, and the scan result would likely
need to continue showing these as separate "BSSes" even though they have
the same BSSID.
> > One example of information that should not be overridden by Beacon
> > frames is WPA and RSN IE since these could differ based on the SSID in
> > multi-SSID configurations. An example of information that must be
> > updated from every Beacon frame would be ERP IE. Finally, there are also
> > more complex cases, like the Capability Information, which should
> > probably be handled separately for each subfield (e.g., Privacy could be
> > per-SSID and Short Slot Time for every SSID of the BSSID).
>
> Right. But wouldn't that be covered by operating on different BSS
> structs?
It is next to impossible to figure out which SSID the Beacon frame is
based on in multi-SSID configuration and as such, it is difficult to say
which BSS structure would be updated.. I think there has to be support
for selective update of information based on Beacon frames, but there
are multiple ways of implementing this.
> > Any information that has to be same for all SSIDs in multi-SSID (single
> > BSSID) configuration can be updated based on Beacon frames since the
> > SSIDs for a specific SSID would not contain any different information
> > for these parts. However, fields that can be configured per-SSID, must
> > be handled separately in order to be able to support multi-SSID
> > configuration with different security policies per SSID. This would
> > mainly included Privacy flag in the capability field and security
> > related IEs (WPA IE, RSN IE), but there may be something else that would
> > benefit of similar handling.
>
> It might help if there was code in hostapd to actually do run this
> scenario. *hint* *hint* :)
Yeah.. Well, the problem is that no one has asked (as far as I know) for
this for a real world use case after multi-BSS support was added. Sure,
this would be nice for testing purposes, but the changes needed for
multi-SSID support are somewhat complex and would not only require quite
a bit of work now, but also make long term maintenance take more
effort..
I'm not completely against doing this, but this is not exactly on top of
my to-do list ;-).
--
Jouni Malinen PGP id EFC895FA
> The Beacon frames would probably need to update all <BSSID,*> entries
> for the fields that we decide need to be updated based on Beacons, not
> just the <BSSID,"">... Alternatively, we could change the BSS table to
> have just one entry for each BSSID and a list of SSID entries for each
> BSSID (with SSID, WPA/RSN IE, Privacy flag, etc. from ProbeResp). This
> would be internal to kernel, though, and the scan result would likely
> need to continue showing these as separate "BSSes" even though they have
> the same BSSID.
Oh. Good point. Any comments on Bill Moss's patch though? At least for
the time being that seems like a reasonable thing to do.
> Yeah.. Well, the problem is that no one has asked (as far as I know) for
> this for a real world use case after multi-BSS support was added. Sure,
> this would be nice for testing purposes, but the changes needed for
> multi-SSID support are somewhat complex and would not only require quite
> a bit of work now, but also make long term maintenance take more
> effort..
>
> I'm not completely against doing this, but this is not exactly on top of
> my to-do list ;-).
Right, makes sense :)
johannes
On Mon, 2008-03-31 at 18:04 +0300, Jouni Malinen wrote:
> On Sun, Mar 30, 2008 at 10:33:19AM +0200, Johannes Berg wrote:
>
> > After having two bug reports about the BSS list/probe_resp variable in
> > mac80211 and a very detailed analysis of the problem from Bill Moss, I
> > think I've understood the problem now but I'm not sure how to proceed
> > because I do not understand the reason for "Do not allow beacons to
> > override data from probe responses".
>
> The main (and only) reason for this is support for multi-SSID
> configurations (hidden SSID and/or multiple SSIDs with the same BSSID,
> but with different security parameters). Only the Probe Response frames
> are guaranteed to include correct information for <BSSID,SSID> pair in
> these cases and because of that, Beacon frames are not allowed to update
> things like SSID or WPA/RSN IE.
Right. But in that case, a beacon frame we receive will update the
<BSSID,""> BSS structure, not the <BSSID,actual SSID> structure because
the update code will find only the former since the SSID is hidden.
TheBSS structure w/o SSID won't actually be used.
> One example of information that should not be overridden by Beacon
> frames is WPA and RSN IE since these could differ based on the SSID in
> multi-SSID configurations. An example of information that must be
> updated from every Beacon frame would be ERP IE. Finally, there are also
> more complex cases, like the Capability Information, which should
> probably be handled separately for each subfield (e.g., Privacy could be
> per-SSID and Short Slot Time for every SSID of the BSSID).
Right. But wouldn't that be covered by operating on different BSS
structs?
> > The only thing I'm not sure about is whether that will introduce
> > regressions because probe responses can contain better information than
> > beacons. The only reason I can find for this would be an AP that has
> > multiple SSIDs with different security settings on each and announces
> > those only in the probe responses (and otherwise uses a hidden SSID),
> > but we only handle that now, much later than the code there that was
> > already present.
>
> I'm not sure whether the current code is correct for all information
> from Beacon/ProbeResp (i.e., some parts might need to be moved to be
> before/after this check), but changes to WPA/RSN IE are after this
> return-on-Beacon for a reason and removing that check would break some
> multi-SSID networks.
Ok, I think the HT for example should be moved in front.
> > However, that actually seems to have another bug, if you have such an AP
> > will you find two BSSes, one with a hidden and one with a visible SSID?
>
> That is by design.
Right, ok.
> I think that we will need to evaluate whether the current implementation
> is updating all pieces of information in correct place and still
> maintain possibility of not using Beacon frames to update some fields.
Right. Show of hands who wants to do this? :) Bill sent me a patch
privately, I've asked him to post it here on the list for comments. I
really don't have time right now to investigate all this.
> There might be cases where the old information from ProbeResp should
> expire (though, if we are still associated with that BSSID,SSID pair, we
> should use Probe Request to update this information instead of just
> blindly throughing it away).
Yeah, sure.
> Any information that has to be same for all SSIDs in multi-SSID (single
> BSSID) configuration can be updated based on Beacon frames since the
> SSIDs for a specific SSID would not contain any different information
> for these parts. However, fields that can be configured per-SSID, must
> be handled separately in order to be able to support multi-SSID
> configuration with different security policies per SSID. This would
> mainly included Privacy flag in the capability field and security
> related IEs (WPA IE, RSN IE), but there may be something else that would
> benefit of similar handling.
It might help if there was code in hostapd to actually do run this
scenario. *hint* *hint* :)
johannes
> > The second problem is the one Bill Moss and Vladimir Koutny were running
> > into. As soon as we receive a probe response from a BSS, we never again
> > update it from beacons, hence userspace can never find an AP during
> > passive scanning if we've seen that BSSID before during an active scan.
[...]
> For example WMM parameters are not present in beacon but are present
> in probe response.
> What is needed here is being selective what is updated and what not.
> What is for sure you need at least one probe response before you can associate.
Hm, isn't that particular information also contained in association
responses? I don't think receiving probe responses is really required,
which IEs do you have in mind that aren't also part of the association
exchange?
It does seem that the current code is not correct if we allow going from
"have IE" to "don't have IE" when updating from beacons, but I don't see
why probe responses should actually be required, and we certainly do not
try to enforce that.
In any case, do you have any idea how to fix this problem? Simply update
things selectively? Do you want to take over handling it? :)
Also, I'm not sure why it shows up with iwlwifi but no other hardware,
does iwlwifi filter out probe responses if it got a beacon for the same
BSSID or something like that?
johannes
John W. Linville wrote:
> On Sun, Mar 30, 2008 at 10:33:19AM +0200, Johannes Berg wrote:
>
>> The first one is that there is no actual expiration of BSS structs. Each
>> BSS struct has a 'last_update' member that contains (in jiffies) the
>> time this item was last updated. This means that we accumulate BSS
>> information forever, but due to the 'last_update' only the last few
>> items will be returned to the user on asking for a scan result. This
>> obviously has problems since a rogue station could bombard us with fake
>> probe responses and cause us to build a huge BSS list which is never
>> again freed until the hardware is deregistered. This will need to be
>> fixed, of course.
>
> Assuming this was fixed, does the rest of this issue go away?
> It seems like it would.
I can just add another bug this is causing, this time for IBSS.
The scenario:
- you happen to have 2 IBSS entries for the same SSID/different BSSID (both
of them inactive at this time; this can happen e.g. with IBSS merge),
- you start new IBSS with the same SSID,
- there is no other IBSS station with this SSID around.
In this case, the stack will end up using those 2 BSSIDs alternately, in 30sec
intervals. The reason is that since there is no other active IBSS STA, it will
look at its BSS list and adopts the BSSID (and other params) of first matching
IBSS that is different to current one (or triggers a new scan, which will never
happen in this scenario; see ieee80211_sta_find_ibss).
This seems to be fine when BSS list expiration is fixed, though (with timeout
of less than 30s (IEEE80211_IBSS_MERGE_INTERVAL)).
Vlado
On Mon, 2008-03-31 at 16:05 -0400, John W. Linville wrote:
> On Sun, Mar 30, 2008 at 10:33:19AM +0200, Johannes Berg wrote:
>
> > The first one is that there is no actual expiration of BSS structs. Each
> > BSS struct has a 'last_update' member that contains (in jiffies) the
> > time this item was last updated. This means that we accumulate BSS
> > information forever, but due to the 'last_update' only the last few
> > items will be returned to the user on asking for a scan result. This
> > obviously has problems since a rogue station could bombard us with fake
> > probe responses and cause us to build a huge BSS list which is never
> > again freed until the hardware is deregistered. This will need to be
> > fixed, of course.
>
> Assuming this was fixed, does the rest of this issue go away?
> It seems like it would.
Sort of, it'll go away as soon as the info is expired, but you still
won't be able to switch over right away.
> Should we consider allowing the beacon to update the info if
> last_update is old enough to keep the BSS out of the scan results?
That, maybe, and maybe also if we don't want to associate with that
network right away or something.
Really, I think this needs somebody to dig into the specs, figure out
which things can change from where, and mostly rewrite it all
appropriately. I for sure won't have time to do that.
johannes