* Re: [PATCH 1/2 V3] nl80211/cfg80211: Add support for drivers with AP SME that require PMF SA Query assistance
@ 2014-01-03 21:30 clanctot
2014-01-06 16:38 ` Johannes Berg
2014-01-07 16:04 ` Johannes Berg
0 siblings, 2 replies; 5+ messages in thread
From: clanctot @ 2014-01-03 21:30 UTC (permalink / raw)
To: Johannes Berg; +Cc: Chet Lanctot, linville, linux-wireless
Johannes,
Thanks for your comments regarding these changes. See my responses below.
- Chet
> On Tue, 2013-12-10 at 14:11 -0800, Chet Lanctot wrote:
>> This adds support for drivers that have AP SME integrated but do not
implement the SA Query procedure that is part of Protected
>> Management Frames (PMF, 802.11w).
>> Instead, hostapd can be used to assist drivers that lack SA Query
Procedure handling on their own by allowing them to specify this as a
device capability flag.
>> Also, a station flag is added to let hostapd indicate to the driver
that the SA Query procedure is complete and the driver can process
association requests from the station normally.
> How will this work? If the device is processing the auth/assoc request
frames, then how can hostapd know this? Is the device expected to then
pass up the frame?
Yes, the device will detect the case of an unprotected Assoc Request from
a station where there is already a PMF connection and, in this case, will
pass up the frame to hostapd. hostapd already has code to handle this
case and will initiate the SA Query procedure. This has been tested and
it works correctly.
The station flag is added for hostapd to signal back to the device that SA
Query has timed out (completed) and that the device should process Assoc
Request frames from that station normally.
> Also, which (upstream) driver is going to use this?
These changes are of general utility for any device that implements AP SME
and will also implement PMF. We will be up-steaming a driver that will
use these extensions in the future.
>> + * @NL80211_ATTR_AP_SME_NO_SA_QUERY: The driver for this device +
* implments the AP SME but lacks support for doing the MFP SA
> typo: implements
I will fix this typo.
>> + * Query procedure. This flag is used to express the need for + * a
userspace helper (hostapd) to do this procedure and notifiy
> typo: notify
I will fix this typo.
>> + * the driver through cfg80211 when it is complete.
> Should probably say how the driver is notified (via the station flag)?
I will clarify this comment.
>> @@ -3689,7 +3689,7 @@ int cfg80211_check_station_change(struct wiphy
*wiphy,
>> return -EINVAL;
>> /* When you run into this, adjust the code below for the new flag */
>> - BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 7);
>> + BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 8);
>> switch (statype) {
>> case CFG80211_STA_MESH_PEER_KERNEL:
>> @@ -3766,7 +3766,8 @@ int cfg80211_check_station_change(struct wiphy
*wiphy,
>> BIT(NL80211_STA_FLAG_ASSOCIATED) |
>> BIT(NL80211_STA_FLAG_SHORT_PREAMBLE) |
>> BIT(NL80211_STA_FLAG_WME) |
>> - BIT(NL80211_STA_FLAG_MFP)))
>> + BIT(NL80211_STA_FLAG_MFP) |
>> + BIT(NL80211_STA_FLAG_NO_SA_QUERY_REQUIRED)))
>> return -EINVAL;
>> /* but authenticated/associated only if driver handles it */
> Maybe we should also check if the driver supports the flag?
For existing PMF (MFP) code, there is no check for driver support of PMF.
wpa_supplicant and hostapd just pass PMF information to the driver. The
driver just ignores the information if it does not support PMF.
>> @@ -4090,7 +4091,7 @@ static int nl80211_new_station(struct sk_buff
*skb, struct genl_info *info)
>> return -EINVAL;
>> /* When you run into this, adjust the code below for the new flag */
>> - BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 7);
>> + BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 8);
>> switch (dev->ieee80211_ptr->iftype) {
>> case NL80211_IFTYPE_AP:
> Can this really be right? Is it simply invalid on a new station? Why
does this even make sense - this is done for AP SME where this is never
invoked?
> Anyway you need to reject adding TDLS peers with this flag, for example,
afaict.
The idea here was that this new flag is used to signal a state transition
for stations for which the SA Query procedure has already started. So,
yes, it is invalid for a new station operation.
But when the driver detects that an SA Query procedure is needed for an
associated station, it passes the Association Request up to hostapd and
remembers that the SA Query is in progress for that station.
This new flag allows hostapd to signal to the driver that SA Query has
completed and the driver can clear the state for that station indicating
that SA Query is in progress. So this flag is only used in the station
change operation.
Perhaps there is a cleaner way to do this? What we are trying to
accomplish is to allow hostapd to signal to the driver that SA Query has
completed and do this using an already existing nl80211/cfg80211 interface
(and just adding a new flag).
> johannes
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 1/2 V3] nl80211/cfg80211: Add support for drivers with AP SME that require PMF SA Query assistance
2014-01-03 21:30 [PATCH 1/2 V3] nl80211/cfg80211: Add support for drivers with AP SME that require PMF SA Query assistance clanctot
@ 2014-01-06 16:38 ` Johannes Berg
2014-01-07 16:04 ` Johannes Berg
1 sibling, 0 replies; 5+ messages in thread
From: Johannes Berg @ 2014-01-06 16:38 UTC (permalink / raw)
To: clanctot; +Cc: linville, linux-wireless
On Fri, 2014-01-03 at 21:30 +0000, clanctot@codeaurora.org wrote:
> Johannes,
>
> Thanks for your comments regarding these changes. See my responses below.
Your responses are practically impossible to read in this interleaving
with my notes. Care to resend with proper quote marks?
johannes
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2 V3] nl80211/cfg80211: Add support for drivers with AP SME that require PMF SA Query assistance
2014-01-03 21:30 [PATCH 1/2 V3] nl80211/cfg80211: Add support for drivers with AP SME that require PMF SA Query assistance clanctot
2014-01-06 16:38 ` Johannes Berg
@ 2014-01-07 16:04 ` Johannes Berg
1 sibling, 0 replies; 5+ messages in thread
From: Johannes Berg @ 2014-01-07 16:04 UTC (permalink / raw)
To: clanctot; +Cc: linville, linux-wireless
[please fix your quoting. I'll reply anyway this time, but still, it's
odd to read if you get linebreaks but the quote marks only apply to the
first line]
On Fri, 2014-01-03 at 21:30 +0000, clanctot@codeaurora.org wrote:
> Yes, the device will detect the case of an unprotected Assoc Request from
> a station where there is already a PMF connection and, in this case, will
> pass up the frame to hostapd. hostapd already has code to handle this
> case and will initiate the SA Query procedure. This has been tested and
> it works correctly.
Documentation needed then.
> The station flag is added for hostapd to signal back to the device that SA
> Query has timed out (completed) and that the device should process Assoc
> Request frames from that station normally.
Seems reasonable I guess.
> > Also, which (upstream) driver is going to use this?
> These changes are of general utility for any device that implements AP SME
> and will also implement PMF. We will be up-steaming a driver that will
> use these extensions in the future.
I'm curious, for what device? :)
> For existing PMF (MFP) code, there is no check for driver support of PMF.
> wpa_supplicant and hostapd just pass PMF information to the driver. The
> driver just ignores the information if it does not support PMF.
Really? But anyway the driver might be confused if it sees a station
change with something changing that doesn't actually do anything for it?
> >> @@ -4090,7 +4091,7 @@ static int nl80211_new_station(struct sk_buff
> *skb, struct genl_info *info)
> >> return -EINVAL;
> >> /* When you run into this, adjust the code below for the new flag */
> >> - BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 7);
> >> + BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 8);
> >> switch (dev->ieee80211_ptr->iftype) {
> >> case NL80211_IFTYPE_AP:
> > Can this really be right? Is it simply invalid on a new station? Why
> does this even make sense - this is done for AP SME where this is never
> invoked?
> > Anyway you need to reject adding TDLS peers with this flag, for example,
> afaict.
> The idea here was that this new flag is used to signal a state transition
> for stations for which the SA Query procedure has already started. So,
> yes, it is invalid for a “new station” operation.
Right, so then you should just reject it there? New station is never
even called for AP SME, I believe, so it wouldn't make sense at all to
specify the flag.
> But when the driver detects that an SA Query procedure is needed for an
> associated station, it passes the Association Request up to hostapd and
> remembers that the SA Query is in progress for that station.
>
> This new flag allows hostapd to signal to the driver that SA Query has
> completed and the driver can clear the state for that station indicating
> that SA Query is in progress. So this flag is only used in the “station
> change” operation.
Yeah, I get that.
> Perhaps there is a cleaner way to do this? What we are trying to
> accomplish is to allow hostapd to signal to the driver that SA Query has
> completed and do this using an already existing nl80211/cfg80211 interface
> (and just adding a new flag).
I think that's OK, but you're adding it in a way that allows the flag to
be used with other operations and that'd be inconsistent with the flag
semantics, it seems?
johannes
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 0/2 V3] nl80211/cfg80211: Support PMF on drivers with integrated AP SME
@ 2013-12-10 22:11 Chet Lanctot
2013-12-10 22:11 ` [PATCH 1/2 V3] nl80211/cfg80211: Add support for drivers with AP SME that require PMF SA Query assistance Chet Lanctot
0 siblings, 1 reply; 5+ messages in thread
From: Chet Lanctot @ 2013-12-10 22:11 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Chet Lanctot
These patches represent a small number of extensions to the
nl80211/cfg80211 interface to support Protected Management Frames
(PMF, 802.11w) on an AP when the AP SME is integrated into the
device driver or device firmware.
Device drivers that implement AP SME handle connection requests
from stations internally. These patches allow hostapd to specify
to the device driver the PMF state that should be used when these
connections are made.
Also, these patches allow device drivers that do not implement the
SA Query procedure (part of PMF) to communicate the need for hostapd
do this procedure. Normally the software component handling
connection requests would do SA Query. These patches provide a way
for the device driver to shift SA Query processing to hostapd where
it is already fully implemented.
The following changes are made to nl80211/cfg80211.
1. A new nl80211_ap_sme_feature is defined which is used by drivers
to inform hostapd that the driver does not support the SA query
procedure. hostapd must register for Re/Association Request frames
from the driver so that these frames can be delivered by the driver
to start an SA Query procedure. An example of code in
the driver that sets this feature is as follows:
struct wiphy *wiphy; /* wiphy defined in cfg80211.h */
.
.
.
wiphy->ap_sme_capa |= BIT(NL80211_AP_SME_FEATURE_NO_SA_QUERY);
2. A new entry is made in cfg80211_ap_settings to inform drivers
whether management frame protection should be used for station connections.
This entry is passed by hostapd using NL80211_CMD_START_AP and it is only
used when the device is acting as an AP. Existing type nl80211_mfp defines the
values that can be used for this entry. Existing value NL80211_MFP_NO
means that PMF connections cannot be made with stations. Existing value
NL80211_MFP_REQUIRED means that all station connections must be PMF
protected. A new value NL80211_MFP_OPTIONAL is defined which means
that a connection can be made if the station supports it, but it is
not required.
3. A new station flag is defined that indicates to the driver that
hostapd has completed the SA Query procedure for that station (SA
Query timed out) and the driver should process the next Re/Association
Request normally and not pass it to hostapd.
Chet Lanctot (2):
nl80211/cfg80211: Add support for drivers with AP SME that require
PMF SA Query assistance
nl80211/cfg80211: Enable station PMF requirement to be specified to
driver with AP SME
include/net/cfg80211.h | 4 ++++
include/uapi/linux/nl80211.h | 26 ++++++++++++++++++++------
net/wireless/nl80211.c | 16 +++++++++++++---
3 files changed, 37 insertions(+), 9 deletions(-)
--
1.7.12.rc0.22.gcdd159b
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/2 V3] nl80211/cfg80211: Add support for drivers with AP SME that require PMF SA Query assistance
2013-12-10 22:11 [PATCH 0/2 V3] nl80211/cfg80211: Support PMF on drivers with integrated AP SME Chet Lanctot
@ 2013-12-10 22:11 ` Chet Lanctot
2013-12-16 14:19 ` Johannes Berg
0 siblings, 1 reply; 5+ messages in thread
From: Chet Lanctot @ 2013-12-10 22:11 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Chet Lanctot
This adds support for drivers that have AP SME integrated but do
not implement the SA Query procedure that is part of Protected
Management Frames (PMF, 802.11w).
Instead, hostapd can be used to assist drivers that lack SA Query
Procedure handling on their own by allowing them to specify this as
a device capability flag.
Also, a station flag is added to let hostapd indicate to the driver
that the SA Query procedure is complete and the driver can process
association requests from the station normally.
Signed-off-by: Chet Lanctot <clanctot@codeaurora.org>
---
include/uapi/linux/nl80211.h | 14 +++++++++++---
net/wireless/nl80211.c | 7 ++++---
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index eb68735..4c80a10 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1891,6 +1891,9 @@ enum nl80211_iftype {
* @NL80211_STA_FLAG_ASSOCIATED: station is associated; used with drivers
* that support %NL80211_FEATURE_FULL_AP_CLIENT_STATE to transition a
* previously added station into associated state
+ * @NL80211_STA_FLAG_NO_SA_QUERY_REQUIRED: hostapd has completed the MFP
+ * SA Query procedure with the station and no further SA Query is needed
+ * when an association request is received from the station
* @NL80211_STA_FLAG_MAX: highest station flag number currently defined
* @__NL80211_STA_FLAG_AFTER_LAST: internal use
*/
@@ -1903,6 +1906,7 @@ enum nl80211_sta_flags {
NL80211_STA_FLAG_AUTHENTICATED,
NL80211_STA_FLAG_TDLS_PEER,
NL80211_STA_FLAG_ASSOCIATED,
+ NL80211_STA_FLAG_NO_SA_QUERY_REQUIRED,
/* keep last */
__NL80211_STA_FLAG_AFTER_LAST,
@@ -3648,11 +3652,15 @@ enum nl80211_tdls_operation {
/*
* enum nl80211_ap_sme_features - device-integrated AP features
- * Reserved for future use, no bits are defined in
- * NL80211_ATTR_DEVICE_AP_SME yet.
+ * @NL80211_ATTR_AP_SME_NO_SA_QUERY: The driver for this device
+ * implments the AP SME but lacks support for doing the MFP SA
+ * Query procedure. This flag is used to express the need for
+ * a userspace helper (hostapd) to do this procedure and notifiy
+ * the driver through cfg80211 when it is complete.
+ */
enum nl80211_ap_sme_features {
+ NL80211_AP_SME_FEATURE_NO_SA_QUERY
};
- */
/**
* enum nl80211_feature_flags - device/driver features
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 587ff84..276e4a3 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3689,7 +3689,7 @@ int cfg80211_check_station_change(struct wiphy *wiphy,
return -EINVAL;
/* When you run into this, adjust the code below for the new flag */
- BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 7);
+ BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 8);
switch (statype) {
case CFG80211_STA_MESH_PEER_KERNEL:
@@ -3766,7 +3766,8 @@ int cfg80211_check_station_change(struct wiphy *wiphy,
BIT(NL80211_STA_FLAG_ASSOCIATED) |
BIT(NL80211_STA_FLAG_SHORT_PREAMBLE) |
BIT(NL80211_STA_FLAG_WME) |
- BIT(NL80211_STA_FLAG_MFP)))
+ BIT(NL80211_STA_FLAG_MFP) |
+ BIT(NL80211_STA_FLAG_NO_SA_QUERY_REQUIRED)))
return -EINVAL;
/* but authenticated/associated only if driver handles it */
@@ -4090,7 +4091,7 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
/* When you run into this, adjust the code below for the new flag */
- BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 7);
+ BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 8);
switch (dev->ieee80211_ptr->iftype) {
case NL80211_IFTYPE_AP:
--
1.7.12.rc0.22.gcdd159b
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 1/2 V3] nl80211/cfg80211: Add support for drivers with AP SME that require PMF SA Query assistance
2013-12-10 22:11 ` [PATCH 1/2 V3] nl80211/cfg80211: Add support for drivers with AP SME that require PMF SA Query assistance Chet Lanctot
@ 2013-12-16 14:19 ` Johannes Berg
0 siblings, 0 replies; 5+ messages in thread
From: Johannes Berg @ 2013-12-16 14:19 UTC (permalink / raw)
To: Chet Lanctot; +Cc: linville, linux-wireless
On Tue, 2013-12-10 at 14:11 -0800, Chet Lanctot wrote:
> This adds support for drivers that have AP SME integrated but do
> not implement the SA Query procedure that is part of Protected
> Management Frames (PMF, 802.11w).
>
> Instead, hostapd can be used to assist drivers that lack SA Query
> Procedure handling on their own by allowing them to specify this as
> a device capability flag.
>
> Also, a station flag is added to let hostapd indicate to the driver
> that the SA Query procedure is complete and the driver can process
> association requests from the station normally.
How will this work? If the device is processing the auth/assoc request
frames, then how can hostapd know this? Is the device expected to then
pass up the frame?
Also, which (upstream) driver is going to use this?
> + * @NL80211_ATTR_AP_SME_NO_SA_QUERY: The driver for this device
> + * implments the AP SME but lacks support for doing the MFP SA
typo: implements
> + * Query procedure. This flag is used to express the need for
> + * a userspace helper (hostapd) to do this procedure and notifiy
typo: notify
> + * the driver through cfg80211 when it is complete.
Should probably say how the driver is notified (via the station flag)?
> @@ -3689,7 +3689,7 @@ int cfg80211_check_station_change(struct wiphy *wiphy,
> return -EINVAL;
>
> /* When you run into this, adjust the code below for the new flag */
> - BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 7);
> + BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 8);
>
> switch (statype) {
> case CFG80211_STA_MESH_PEER_KERNEL:
> @@ -3766,7 +3766,8 @@ int cfg80211_check_station_change(struct wiphy *wiphy,
> BIT(NL80211_STA_FLAG_ASSOCIATED) |
> BIT(NL80211_STA_FLAG_SHORT_PREAMBLE) |
> BIT(NL80211_STA_FLAG_WME) |
> - BIT(NL80211_STA_FLAG_MFP)))
> + BIT(NL80211_STA_FLAG_MFP) |
> + BIT(NL80211_STA_FLAG_NO_SA_QUERY_REQUIRED)))
> return -EINVAL;
>
> /* but authenticated/associated only if driver handles it */
Maybe we should also check if the driver supports the flag?
> @@ -4090,7 +4091,7 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
> return -EINVAL;
>
> /* When you run into this, adjust the code below for the new flag */
> - BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 7);
> + BUILD_BUG_ON(NL80211_STA_FLAG_MAX != 8);
>
> switch (dev->ieee80211_ptr->iftype) {
> case NL80211_IFTYPE_AP:
Can this really be right? Is it simply invalid on a new station? Why
does this even make sense - this is done for AP SME where this is never
invoked?
Anyway you need to reject adding TDLS peers with this flag, for example,
afaict.
johannes
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-01-07 16:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-03 21:30 [PATCH 1/2 V3] nl80211/cfg80211: Add support for drivers with AP SME that require PMF SA Query assistance clanctot
2014-01-06 16:38 ` Johannes Berg
2014-01-07 16:04 ` Johannes Berg
-- strict thread matches above, loose matches on Subject: below --
2013-12-10 22:11 [PATCH 0/2 V3] nl80211/cfg80211: Support PMF on drivers with integrated AP SME Chet Lanctot
2013-12-10 22:11 ` [PATCH 1/2 V3] nl80211/cfg80211: Add support for drivers with AP SME that require PMF SA Query assistance Chet Lanctot
2013-12-16 14:19 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).