Hidden SSID is a mess right now. Here's why, and here's a proposal for
fixing it.
Background:
-----------
There are two settings for ap_scan with wpa_supplicant. ap_scan=1 along
with scan_ssid=1 in a network block lets wpa_supplicant tell the driver
to probe for the SSID it wants to connect to, exposing hidden SSIDs to
the supplicant and allowing it to successfully connect.
ap_scan=2 just blows the settings to the driver via WEXT, and hopes the
driver can do the right thing. That's problematic for a few reasons;
namely that WEXT doesn't have the concept of "packets" of association
settings (so drivers have to guess which settings to apply together and
which not to based on timeouts, or else just fail here entirely), and
that ordering relationships between WEXT commands are ambiguous. This
leads to failure.
ap_scan=2 _should_ be supported by all drivers, because it is the
brute-force fallback. Unfortunately, iwlwifi drivers are currently
broken with hidden SSIDs and ap_scan=2, and maybe all mac80211 drivers
too. Regardless of whether ap_scan=2 should work or not, drivers that
support specific SSID scans can do better. But keep in mind that
ap_scan=2 is ALWAYS the fallback and should ALWAYS work with any driver.
Drivers that can't do specific SSID scans mostly don't work with
ap_scan=1 because they usually don't do probe requests on command.
Some drivers (most fullmac ones) require ap_scan=2 for hidden SSID
support. Those would include orinoco, airo, atmel, and ipw2100.
Other drivers (mostly softmac ones) work well with ap_scan=1 because
they support scanning for specific SSIDs.
However, userspace is f*cked because it can't figure out which one to
use, and trying both is not an option because you get unacceptable
latency for the user when trying to associate. And special-casing based
on the driver name is NOT an option. Ever. At all.
Solution
--------
Drivers should advertise the ability to do specific SSID scans (which
allows them to find hidden SSIDs). As a requirement for supporting this
capability, the driver _MUST_ implement the required bits in its
SIOCSIWSCAN handler to scan for the requested SSID, or else the driver
is in error.
Userspace apps can then intelligently determine whether to use ap_scan=1
or ap_scan=2 with wpa_supplicant. If the driver is not fixed to
advertise this capability, userspace can fall back to ap_scan=2 and the
driver is clearly the problem in this case, and the responsible piece of
code can be blamed and fixed.
The problem is that there isn't a nice, generic capabilities field in
WEXT. So we have to abuse an existing one, and I picked enc_capa.
Notes
-----
I know that the ap_scan=1 or 2 is really supposed to be for selecting
roaming behavior. However, it's also overloaded in wpa_supplicant to
control hidden SSID handling as a workaround for drivers not supporting
specific SSID scans. Other utilities have to be able to figure out
which one to use for hidden SSIDs.
Obviously cfg80211/net80211 would be able to export this capability
directly in the capability bits for each driver. This issue is only a
problem with WEXT (unless cfg80211 doesn't have this capability yet).
The patch below is just to show how this might be accomplished.
Obviously mac80211 drivers have this capability. So does ipw2200.
ipw2100 does not have this capability, and therefore does not set the
bit in enc_capa.
Thoughts? It's at the point where something just needs to be done to
fix the situation.
Dan
diff --git a/drivers/net/wireless/ipw2200.c b/drivers/net/wireless/ipw2200.c
index 54f44e5..726e30e 100644
--- a/drivers/net/wireless/ipw2200.c
+++ b/drivers/net/wireless/ipw2200.c
@@ -8901,6 +8901,8 @@ static int ipw_wx_get_range(struct net_device *dev,
range->enc_capa = IW_ENC_CAPA_WPA | IW_ENC_CAPA_WPA2 |
IW_ENC_CAPA_CIPHER_TKIP | IW_ENC_CAPA_CIPHER_CCMP;
+ range->enc_capa |= IW_ENC_CAPA_SPECIFIC_SSID_SCAN;
+
IPW_DEBUG_WX("GET Range\n");
return 0;
}
diff --git a/include/linux/wireless.h b/include/linux/wireless.h
index 0987aa7..e2a8fd8 100644
--- a/include/linux/wireless.h
+++ b/include/linux/wireless.h
@@ -625,6 +625,10 @@
#define IW_ENC_CAPA_CIPHER_TKIP 0x00000004
#define IW_ENC_CAPA_CIPHER_CCMP 0x00000008
+/* Abuse enc_capa because there is no generic capabilties
+ * field in the range struct. */
+#define IW_ENC_CAPA_SPECIFIC_SSID_SCAN 0x00000010
+
/* Event capability macros - in (struct iw_range *)->event_capa
* Because we have more than 32 possible events, we use an array of
* 32 bit bitmasks. Note : 32 bits = 0x20 = 2^5. */
diff --git a/net/mac80211/ieee80211_ioctl.c b/net/mac80211/ieee80211_ioctl.c
index 646e2f2..fd842c4 100644
--- a/net/mac80211/ieee80211_ioctl.c
+++ b/net/mac80211/ieee80211_ioctl.c
@@ -190,6 +190,9 @@ static int ieee80211_ioctl_giwrange(struct net_device *dev,
range->enc_capa = IW_ENC_CAPA_WPA | IW_ENC_CAPA_WPA2 |
IW_ENC_CAPA_CIPHER_TKIP | IW_ENC_CAPA_CIPHER_CCMP;
+ /* All mac80211 drivers support this capability */
+ range->enc_capa |= IW_ENC_CAPA_SPECIFIC_SSID_SCAN;
+
list_for_each_entry(mode, &local->modes_list, list) {
int i = 0;
On Wed, Dec 05, 2007 at 05:44:32PM -0500, Dan Williams wrote:
>
> > > The problem is that there isn't a nice, generic capabilities field in
> > > WEXT. So we have to abuse an existing one, and I picked enc_capa.
> >
> > Please don't do that. It's fairly trivial to add a new field
> > in iwrange. And it's not like the sky will fall down if we add a field
> > in iwrange. If you want to do it, please do it right.
>
> Well, the problem I had with this is that it's less likely to get
> backported because it potentially breaks binary compatibility for
> distros. There are 3 options here:
>
> 1) overload enc_capa
>
> 2) add a field and start doing comparisons of the size of the iw_range
> structure to figure out what is available and what's not
>
> 3) Bump the WEXT version
>
> #3 is a non-starter because it is a much larger, more invasive change
> and requires more breakage of ABI and such.
Hu ? Please tell me what will break. We can bump the WEXT
version pretty much any time we want, no userspace will fail. THe
worse you may see is an anoying warning message.
> I guess #2 is acceptable.
Option 4 : use two flags (as described below), that way you
know if iwrange implement the feature or not.
> > Note that there is one gotcha because of the state of
> > userspace, so if you want I can produce a kernel patch that will work
> > properly, and I'll add the usespace support for it as well.
>
> Could you describe that more?
The iwrange in userspace has a few more stuff defined, so you
need to make sure to include padding to not add your field on an
already existing field. Check the wireless.21.h file.
Note that if you sign the usual suspects on that change, I'll
do the patch for you.
> > That won't work. You have a tri-state, and with only a single
> > bit you can encode only two state. You basically have :
> > o driver that support ap_scan=1
> > o driver that do not support ap_scan=1
> > o driver that have not been updated or reside in old kernels
>
> Well, to be fair, the patch posted handles this perfectly well, because
> older kernels with drivers that don't have the capability just dont'
> advertise it. Therefore everything works fine.
>
> > So, at the minimum, you'd want :
> > -------------------------------------
> > +#define IW_ENC_CAPA_SPECIFIC_SSID_SCAN 0x00000010
> > +#define IW_ENC_CAPA_ANY_SSID_SCAN 0x00000020
> > -------------------------------------
>
> I'm not sure why we'd want ANY_SSID_SCAN?
To distinguish driver that have not been updated or driver in
order kernel from drivers that do not support ap_scan=1. You may want
to do things slightly differently between those two.
It just act as : yes this driver has been audited and fixed to
export proper scanning capability support.
> > Another option would be for SIOCSIWSCAN to fail if a specific
> > SSID is set and it does not support it, instead of just ignoring it.
>
> Again, that won't work because it requires changing _all_ drivers.
And ? In some case it's preferable to fix all driver rather
than to mess us the API. In many instances we fix all drivers.
> We
> need to fix this by ensuring that the change only affects those drivers
> that support the capability, not requiring drivers that dont' have it to
> change. Extending range would work here.
>
> Dan
Have fun,
Jean
On Thu, Dec 06, 2007 at 06:02:35AM -0500, Dan Williams wrote:
>
> It's a problem for backporting this fix to older kernels that are not
> 2.6.25.
I think backporting would be a non starter, only bug fixes are
suppossed to be backported. For out-of-tree driver, a #ifdef will do
the trick trivially.
> So is the ANY_SSID_SCAN thing different from just calling SIOCSIWSCAN
> _without_ any options? I was under the impression that just calling
> SIWSCAN would scan for all SSIDs.
You are right. Actually, if you stuff a empty essid in
SIOCSIWSCAN, I doubt it would work.
The purpose of the flag is different : identify audited from
non audited drivers.
> I've actually thought about your proposal, and decided that you're
> right. I'll post another patch for more comments.
>
> dan
Regards,
Jean
Dan Williams wrote :
>
> Hidden SSID is a mess right now. Here's why, and here's a proposal for
> fixing it.
If you want my participation, it's usually wise to cc me.
> The problem is that there isn't a nice, generic capabilities field in
> WEXT. So we have to abuse an existing one, and I picked enc_capa.
Please don't do that. It's fairly trivial to add a new field
in iwrange. And it's not like the sky will fall down if we add a field
in iwrange. If you want to do it, please do it right.
Note that there is one gotcha because of the state of
userspace, so if you want I can produce a kernel patch that will work
properly, and I'll add the usespace support for it as well.
> +#define IW_ENC_CAPA_SPECIFIC_SSID_SCAN 0x00000010
That won't work. You have a tri-state, and with only a single
bit you can encode only two state. You basically have :
o driver that support ap_scan=1
o driver that do not support ap_scan=1
o driver that have not been updated or reside in old kernels
So, at the minimum, you'd want :
-------------------------------------
+#define IW_ENC_CAPA_SPECIFIC_SSID_SCAN 0x00000010
+#define IW_ENC_CAPA_ANY_SSID_SCAN 0x00000020
-------------------------------------
> However, userspace is f*cked because it can't figure out which one to
> use, and trying both is not an option because you get unacceptable
> latency for the user when trying to associate.
Another option would be for SIOCSIWSCAN to fail if a specific
SSID is set and it does not support it, instead of just ignoring it.
Regards,
Jean
On Dec 5, 2007 4:20 PM, Dan Williams <[email protected]> wrote:
> ap_scan=2 just blows the settings to the driver via WEXT, and hopes the
> driver can do the right thing.
Can you elaborate here on how it just 'blows' the settings to the
driver via WEXT?
> However, userspace is f*cked because it can't figure out which one to
> use, and trying both is not an option because you get unacceptable
> latency for the user when trying to associate. And special-casing based
> on the driver name is NOT an option. Ever. At all.
Userspace has been fucked for many reasons. The right solution was to
abandon WE so we should not use it anymore for new features.
> Solution
> --------
>
> Drivers should advertise the ability to do specific SSID scans (which
> allows them to find hidden SSIDs). As a requirement for supporting this
> capability, the driver _MUST_ implement the required bits in its
> SIOCSIWSCAN handler to scan for the requested SSID, or else the driver
> is in error.
Agreed.
> Userspace apps can then intelligently determine whether to use ap_scan=1
> or ap_scan=2 with wpa_supplicant. If the driver is not fixed to
> advertise this capability, userspace can fall back to ap_scan=2 and the
> driver is clearly the problem in this case, and the responsible piece of
> code can be blamed and fixed.
Sure.
> The problem is that there isn't a nice, generic capabilities field in
> WEXT. So we have to abuse an existing one, and I picked enc_capa.
This is what we keep doing and this is why WEXT has become a big pile
of hacks, and precisely for this reason is why we should abandon it.
Lets please not abuse it any further and instead help try to uplift
cfg80211 and nl80211.
> Obviously cfg80211/net80211 would be able to export this capability
> directly in the capability bits for each driver. This issue is only a
> problem with WEXT (unless cfg80211 doesn't have this capability yet).
cfg80211/nl80211 (net80211 is FBS's wireless stack) doesn't export
this in any way yet.
> The patch below is just to show how this might be accomplished.
> Obviously mac80211 drivers have this capability. So does ipw2200.
> ipw2100 does not have this capability, and therefore does not set the
> bit in enc_capa.
>
> Thoughts? It's at the point where something just needs to be done to
> fix the situation.
I believe the right solution here is to ignore WE and instead focus on
cfg80211/nl80211. Since you are willing to modify drivers for this
capability lets instead add cfg80211_ops to those drivers which you
want to fix and add a new NL80211_CMD_GET_SCAN_CAP or the like. Its
probably a bit more effort but its in the right direction and will
help us advance and embrace new the new framework laid out for us.
As for compatibility -- I've added nl80211 now as part of the
compat-wireless-2.6 package, which means kernels >= 2.6.22 can reap
benefits from it
Luis
On Wed, 2007-12-05 at 15:05 -0800, Jean Tourrilhes wrote:
> On Wed, Dec 05, 2007 at 05:44:32PM -0500, Dan Williams wrote:
> >
> > > > The problem is that there isn't a nice, generic capabilities field in
> > > > WEXT. So we have to abuse an existing one, and I picked enc_capa.
> > >
> > > Please don't do that. It's fairly trivial to add a new field
> > > in iwrange. And it's not like the sky will fall down if we add a field
> > > in iwrange. If you want to do it, please do it right.
> >
> > Well, the problem I had with this is that it's less likely to get
> > backported because it potentially breaks binary compatibility for
> > distros. There are 3 options here:
> >
> > 1) overload enc_capa
> >
> > 2) add a field and start doing comparisons of the size of the iw_range
> > structure to figure out what is available and what's not
> >
> > 3) Bump the WEXT version
> >
> > #3 is a non-starter because it is a much larger, more invasive change
> > and requires more breakage of ABI and such.
>
> Hu ? Please tell me what will break. We can bump the WEXT
> version pretty much any time we want, no userspace will fail. THe
> worse you may see is an anoying warning message.
It's a problem for backporting this fix to older kernels that are not
2.6.25. If a WEXT version bump is required, then you have two options
to backport this to drivers:
1) take the entire driver as-is. This usually won't work because many
drivers are in a lot of flux, especially the softmac ones with their
interdependencies on the stack.
2) Backport the SCAN_CAPA stuff only and bump the wext version. This
may mean that the driver says it supports WEXT 24 for example, but
doesn't actually have all the features that the current kernel driver
that advertises v24 has. Another dead end.
> > I guess #2 is acceptable.
>
> Option 4 : use two flags (as described below), that way you
> know if iwrange implement the feature or not.
>
> > > Note that there is one gotcha because of the state of
> > > userspace, so if you want I can produce a kernel patch that will work
> > > properly, and I'll add the usespace support for it as well.
> >
> > Could you describe that more?
>
> The iwrange in userspace has a few more stuff defined, so you
> need to make sure to include padding to not add your field on an
> already existing field. Check the wireless.21.h file.
> Note that if you sign the usual suspects on that change, I'll
> do the patch for you.
>
> > > That won't work. You have a tri-state, and with only a single
> > > bit you can encode only two state. You basically have :
> > > o driver that support ap_scan=1
> > > o driver that do not support ap_scan=1
> > > o driver that have not been updated or reside in old kernels
> >
> > Well, to be fair, the patch posted handles this perfectly well, because
> > older kernels with drivers that don't have the capability just dont'
> > advertise it. Therefore everything works fine.
> >
> > > So, at the minimum, you'd want :
> > > -------------------------------------
> > > +#define IW_ENC_CAPA_SPECIFIC_SSID_SCAN 0x00000010
> > > +#define IW_ENC_CAPA_ANY_SSID_SCAN 0x00000020
> > > -------------------------------------
> >
> > I'm not sure why we'd want ANY_SSID_SCAN?
>
> To distinguish driver that have not been updated or driver in
> order kernel from drivers that do not support ap_scan=1. You may want
> to do things slightly differently between those two.
> It just act as : yes this driver has been audited and fixed to
> export proper scanning capability support.
So is the ANY_SSID_SCAN thing different from just calling SIOCSIWSCAN
_without_ any options? I was under the impression that just calling
SIWSCAN would scan for all SSIDs.
I've actually thought about your proposal, and decided that you're
right. I'll post another patch for more comments.
dan
On Wed, 2007-12-05 at 14:11 -0800, Jean Tourrilhes wrote:
> Dan Williams wrote :
> >
> > Hidden SSID is a mess right now. Here's why, and here's a proposal for
> > fixing it.
>
> If you want my participation, it's usually wise to cc me.
Unintentional omission, sorry... I was thinking of it more as a driver
issue but you're right.
> > The problem is that there isn't a nice, generic capabilities field in
> > WEXT. So we have to abuse an existing one, and I picked enc_capa.
>
> Please don't do that. It's fairly trivial to add a new field
> in iwrange. And it's not like the sky will fall down if we add a field
> in iwrange. If you want to do it, please do it right.
Well, the problem I had with this is that it's less likely to get
backported because it potentially breaks binary compatibility for
distros. There are 3 options here:
1) overload enc_capa
2) add a field and start doing comparisons of the size of the iw_range
structure to figure out what is available and what's not
3) Bump the WEXT version
#3 is a non-starter because it is a much larger, more invasive change
and requires more breakage of ABI and such.
I guess #2 is acceptable.
> Note that there is one gotcha because of the state of
> userspace, so if you want I can produce a kernel patch that will work
> properly, and I'll add the usespace support for it as well.
Could you describe that more?
> > +#define IW_ENC_CAPA_SPECIFIC_SSID_SCAN 0x00000010
>
> That won't work. You have a tri-state, and with only a single
> bit you can encode only two state. You basically have :
> o driver that support ap_scan=1
> o driver that do not support ap_scan=1
> o driver that have not been updated or reside in old kernels
Well, to be fair, the patch posted handles this perfectly well, because
older kernels with drivers that don't have the capability just dont'
advertise it. Therefore everything works fine.
> So, at the minimum, you'd want :
> -------------------------------------
> +#define IW_ENC_CAPA_SPECIFIC_SSID_SCAN 0x00000010
> +#define IW_ENC_CAPA_ANY_SSID_SCAN 0x00000020
> -------------------------------------
I'm not sure why we'd want ANY_SSID_SCAN? It's already assumed that all
drivers support scanning. Drivers that don't support scanning and are
already in the tree have so few users and are so old that we just don't
care. New drivers will get a HUGE NAK if they don't have scanning
support, so I don't really see this as necessary.
> > However, userspace is f*cked because it can't figure out which one to
> > use, and trying both is not an option because you get unacceptable
> > latency for the user when trying to associate.
>
> Another option would be for SIOCSIWSCAN to fail if a specific
> SSID is set and it does not support it, instead of just ignoring it.
Again, that won't work because it requires changing _all_ drivers. We
need to fix this by ensuring that the change only affects those drivers
that support the capability, not requiring drivers that dont' have it to
change. Extending range would work here.
Dan