[PATCH 0/2 V3] nl80211/cfg80211: Support PMF on drivers with integrated AP SME

[PATCH 0/2 V3] nl80211/cfg80211: Support PMF on drivers with integrated AP SME

[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

[PATCH 1/2 V3] nl80211/cfg80211: Add support for drivers with AP SME that require PMF SA Query assistance

[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

Re: [PATCH 1/2 V3] nl80211/cfg80211: Add support for drivers with AP SME that require PMF SA Query assistance

[Johannes Berg]

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

[PATCH 2/2 V3] nl80211/cfg80211: Enable station PMF requirement to be specified to driver with AP SME

[Chet Lanctot]

When the device driver implements the AP SME there is a need
for userspace to indicate to the driver the PMF (Protected
Management Frames, 802.11w) requirements for station connections.

The driver enforces the requested PMF state when processing station
connection requests.  Value NL80211_MFP_NO means that PMF connections
cannot be made with stations.  Value NL80211_MFP_REQUIRED means that all
station connections must be PMF protected.  Value NL80211_MFP_OPTIONAL
means that a connection can be made if the station supports it, but it is not
required.

Signed-off-by: Chet Lanctot <clanctot@codeaurora.org>
---
 include/net/cfg80211.h       |  4 ++++
 include/uapi/linux/nl80211.h | 12 +++++++++---
 net/wireless/nl80211.c       |  9 +++++++++
 3 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index aeaf6df..9039888 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -636,6 +636,9 @@ struct cfg80211_acl_data {
  *	user space)
  * @ssid_len: length of @ssid
  * @hidden_ssid: whether to hide the SSID in Beacon/Probe Response frames
+ * @mfp: indicate whether management frame protection is used for
+ *	station connections, this is enforced by the driver when a station
+ *	attempts to make a connection (see definion of nl80211_mfp for details)
  * @crypto: crypto settings
  * @privacy: the BSS uses privacy
  * @auth_type: Authentication type (algorithm)
@@ -655,6 +658,7 @@ struct cfg80211_ap_settings {
 	const u8 *ssid;
 	size_t ssid_len;
 	enum nl80211_hidden_ssid hidden_ssid;
+	enum nl80211_mfp mfp;
 	struct cfg80211_crypto_settings crypto;
 	bool privacy;
 	enum nl80211_auth_type auth_type;
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 4c80a10..59d4d2e 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -1063,8 +1063,8 @@ enum nl80211_commands {
  *
  * @NL80211_ATTR_USE_MFP: Whether management frame protection (IEEE 802.11w) is
  *	used for the association (&enum nl80211_mfp, represented as a u32);
- *	this attribute can be used
- *	with %NL80211_CMD_ASSOCIATE and %NL80211_CMD_CONNECT requests
+ *	this attribute can be used with %NL80211_CMD_ASSOCIATE,
+ *	%NL80211_CMD_CONNECT, and @NL80211_CMD_START_AP requests
  *
  * @NL80211_ATTR_STA_FLAGS2: Attribute containing a
  *	&struct nl80211_sta_flag_update.
@@ -2934,12 +2934,18 @@ enum nl80211_key_type {
 
 /**
  * enum nl80211_mfp - Management frame protection state
- * @NL80211_MFP_NO: Management frame protection not used
+ * @NL80211_MFP_NO: Management frame protection not used on
+ *	any connection
  * @NL80211_MFP_REQUIRED: Management frame protection required
+ *	on all connections
+ * @NL80211_MFP_OPTIONAL: For an AP, management frame
+ *	protection is optional for a station connection depending
+ *	on whether the station supports MFP
  */
 enum nl80211_mfp {
 	NL80211_MFP_NO,
 	NL80211_MFP_REQUIRED,
+	NL80211_MFP_OPTIONAL,
 };
 
 enum nl80211_wpa_versions {
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 276e4a3..7a73adf 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3207,6 +3207,15 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 			return PTR_ERR(params.acl);
 	}
 
+	if (info->attrs[NL80211_ATTR_USE_MFP]) {
+		params.mfp = nla_get_u32(info->attrs[NL80211_ATTR_USE_MFP]);
+		if (params.mfp != NL80211_MFP_REQUIRED &&
+		    params.mfp != NL80211_MFP_OPTIONAL &&
+		    params.mfp != NL80211_MFP_NO)
+			return -EINVAL;
+	} else
+		params.mfp = NL80211_MFP_NO;
+
 	err = rdev_start_ap(rdev, dev, &params);
 	if (!err) {
 		wdev->preset_chandef = params.chandef;
-- 
1.7.12.rc0.22.gcdd159b

Re: [PATCH 2/2 V3] nl80211/cfg80211: Enable station PMF requirement to be specified to driver with AP SME

[Johannes Berg]

On Tue, 2013-12-10 at 14:11 -0800, Chet Lanctot wrote:

> +	if (info->attrs[NL80211_ATTR_USE_MFP]) {
> +		params.mfp = nla_get_u32(info->attrs[NL80211_ATTR_USE_MFP]);
> +		if (params.mfp != NL80211_MFP_REQUIRED &&
> +		    params.mfp != NL80211_MFP_OPTIONAL &&
> +		    params.mfp != NL80211_MFP_NO)
> +			return -EINVAL;
> +	} else
> +		params.mfp = NL80211_MFP_NO;

Code style.

Wouldn't a feature flag be needed?

johannes

Re: [PATCH 2/2 V3] nl80211/cfg80211: Enable station PMF requirement to be specified to driver with AP SME

[clanctot]

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:
>> +	if (info->attrs[NL80211_ATTR_USE_MFP]) {
>> +		params.mfp = nla_get_u32(info->attrs[NL80211_ATTR_USE_MFP]); +		if
(params.mfp != NL80211_MFP_REQUIRED &&
>> +		    params.mfp != NL80211_MFP_OPTIONAL &&
>> +		    params.mfp != NL80211_MFP_NO)
>> +			return -EINVAL;
>> +	} else
>> +		params.mfp = NL80211_MFP_NO;
> Code style.
> Wouldn't a feature flag be needed?
> johannes

For the issue of code style, I am not sure what you mean here.  I copied
this code from the nl80211_connect routine, which has similar processing
for a “use MFP” parameter.  I tried to make the code I added be consistent
with this existing code.

If you explain what needs to be changed to improve the code style, I will
make the changes.

Regarding your question about a feature flag, there is already a  flag
that indicates that the driver implements the AP SME:
     * @WIPHY_FLAG_HAVE_AP_SME: device integrates AP SME
Is this the type of flag you were referring to?

I am ready to respond to any follow-on comments you have for this section
of code.

Re: [PATCH 2/2 V3] nl80211/cfg80211: Enable station PMF requirement to be specified to driver with AP SME

[Johannes Berg]

On Fri, 2014-01-03 at 21:30 +0000, clanctot@codeaurora.org wrote:

> >> +	} else
> >> +		params.mfp = NL80211_MFP_NO;
> > Code style.

> For the issue of code style, I am not sure what you mean here.  I copied
> this code from the nl80211_connect routine, which has similar processing
> for a “use MFP” parameter.  I tried to make the code I added be consistent
> with this existing code.
> 
> If you explain what needs to be changed to improve the code style, I will
> make the changes.

All branches should (now) have braces if one of them requires it.

> Regarding your question about a feature flag, there is already a  flag
> that indicates that the driver implements the AP SME:
>      * @WIPHY_FLAG_HAVE_AP_SME: device integrates AP SME
> Is this the type of flag you were referring to?

Yes, but it seems to me that you need a flag in the nl80211 API
indicating that the AP SME supports PMF in this way.

johannes