diff mbox

[5/9] cfg80211/nl80211: add authorized flag to roaming event

Message ID 20170426075854.13546-6-luca@coelho.fi (mailing list archive)
State Changes Requested
Delegated to: Johannes Berg
Headers show

Commit Message

Luca Coelho April 26, 2017, 7:58 a.m. UTC
From: Avraham Stern <avraham.stern@intel.com>

Drivers that initiate roaming while being connected to a network that
uses 802.1X authentication need to inform user space if 802.1X
authentication is further required after roaming.
For example, when using the Fast transition protocol, roaming within
the mobility domain does not require new 802.1X authentication, but
roaming to another mobility domain does.
In addition, some drivers may not support 802.1X authentication
(so it has to be done in user space), while other drivers do.

Add a flag to the roaming notification to indicate if user space is
required to do 802.1X authentication after the roaming or not.
This flag will only be used for networks that use 802.1X
authentication. For networks that do not use 802.1X authentication it
is assumed that no further action is required from user space after
the roaming notification.

Signed-off-by: Avraham Stern <avraham.stern@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
---
 include/net/cfg80211.h       |  4 ++++
 include/uapi/linux/nl80211.h | 14 ++++++++++++++
 net/wireless/nl80211.c       |  4 +++-
 net/wireless/sme.c           |  1 +
 4 files changed, 22 insertions(+), 1 deletion(-)

Comments

Arend van Spriel April 26, 2017, 10:05 a.m. UTC | #1
Almost overlooked this one. Thanks for the hint, Johannes.

On 4/26/2017 9:58 AM, Luca Coelho wrote:
> From: Avraham Stern <avraham.stern@intel.com>
> 
> Drivers that initiate roaming while being connected to a network that
> uses 802.1X authentication need to inform user space if 802.1X
> authentication is further required after roaming.
> For example, when using the Fast transition protocol, roaming within
> the mobility domain does not require new 802.1X authentication, but
> roaming to another mobility domain does.

Not sure about the terminology here. Is "mobility domain" the same as 
"ESS" which stands for extended service set as definced in 802.11 
standard. If so, I would prefer use of that term here.

> In addition, some drivers may not support 802.1X authentication
> (so it has to be done in user space), while other drivers do.
> 
> Add a flag to the roaming notification to indicate if user space is
> required to do 802.1X authentication after the roaming or not.
> This flag will only be used for networks that use 802.1X
> authentication. For networks that do not use 802.1X authentication it
> is assumed that no further action is required from user space after
> the roaming notification.
 >
> Signed-off-by: Avraham Stern <avraham.stern@intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
> ---
>   include/net/cfg80211.h       |  4 ++++
>   include/uapi/linux/nl80211.h | 14 ++++++++++++++
>   net/wireless/nl80211.c       |  4 +++-
>   net/wireless/sme.c           |  1 +
>   4 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 115f6fc5a34d..f9f4fde2dc09 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -5384,6 +5384,9 @@ cfg80211_connect_timeout(struct net_device *dev, const u8 *bssid,
>    * @req_ie_len: association request IEs length
>    * @resp_ie: association response IEs (may be %NULL)
>    * @resp_ie_len: assoc response IEs length
> + * @authorized: true if the 802.1X authentication was done by the driver or is
> + *	not needed (e.g., when Fast Transition protocol was used), false
> + *	otherwise. Ignored for networks that don't use 802.1X authentication.

It is not ignored in this patch so it is expected user-space behavior 
you are describing, which is not really needed here in cfg80211 driver api.

>    */
>   struct cfg80211_roam_info {
>   	struct ieee80211_channel *channel;
> @@ -5393,6 +5396,7 @@ struct cfg80211_roam_info {
>   	size_t req_ie_len;
>   	const u8 *resp_ie;
>   	size_t resp_ie_len;
> +	bool authorized;
>   };
>   
>   /**
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index 6095a6c4c412..7bdbce7c4147 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -546,6 +546,12 @@
>    *	well to remain backwards compatible.
>    * @NL80211_CMD_ROAM: request that the card roam (currently not implemented),

Do we want to keep this comment about the request scenario. Is it likely 
implemented soon/ever?

>    *	sent as an event when the card/driver roamed by itself.
> + *	When used as an event, and the driver roamed in a network that requires
> + *	802.1X authentication, %NL80211_ATTR_CONNECTION_AUTHORIZED should be set
> + *	if the 802.1X authentication was done by the driver or if roaming was
> + *	done using Fast Transition protocol (in which case 802.1X authentication
> + *	is not needed). If %NL80211_ATTR_CONNECTION_AUTHORIZED is not set,
> + *	user space is responsible for the 802.1X authentication.

Would you consider using NL80211_ATTR_PORT_AUTHORIZED instead referring 
to the 802.1X port entities.

Regards,
Arend

>    * @NL80211_CMD_DISCONNECT: drop a given connection; also used to notify
>    *	userspace that a connection was dropped by the AP or due to other
>    *	reasons, for this the %NL80211_ATTR_DISCONNECTED_BY_AP and
> @@ -2081,6 +2087,12 @@ enum nl80211_commands {
>    * @NL80211_ATTR_PMK: PMK for the PMKSA identified by %NL80211_ATTR_PMKID.
>    *	This is used with @NL80211_CMD_SET_PMKSA.
>    *
> + * @NL80211_ATTR_CONNECTION_AUTHORIZED: A flag attribute used with
> + *	%NL80211_CMD_ROAM to indicate that 802.1X authentication was done by the
> + *	driver or is not needed (because roaming used the Fast Transition
> + *	protocol). Only valid for roaming in networks that require 802.1X
> + *	authentication.
> + *
Arend van Spriel April 26, 2017, 6:44 p.m. UTC | #2
+ Jouni

On 26-4-2017 12:05, Arend van Spriel wrote:
> Almost overlooked this one. Thanks for the hint, Johannes.
> 
> On 4/26/2017 9:58 AM, Luca Coelho wrote:
>> From: Avraham Stern <avraham.stern@intel.com>
>>
>> Drivers that initiate roaming while being connected to a network that
>> uses 802.1X authentication need to inform user space if 802.1X
>> authentication is further required after roaming.
>> For example, when using the Fast transition protocol, roaming within
>> the mobility domain does not require new 802.1X authentication, but
>> roaming to another mobility domain does.
> 
> Not sure about the terminology here. Is "mobility domain" the same as
> "ESS" which stands for extended service set as definced in 802.11
> standard. If so, I would prefer use of that term here.
> 
>> In addition, some drivers may not support 802.1X authentication
>> (so it has to be done in user space), while other drivers do.
>>
>> Add a flag to the roaming notification to indicate if user space is
>> required to do 802.1X authentication after the roaming or not.
>> This flag will only be used for networks that use 802.1X
>> authentication. For networks that do not use 802.1X authentication it
>> is assumed that no further action is required from user space after
>> the roaming notification.
>>
>> Signed-off-by: Avraham Stern <avraham.stern@intel.com>
>> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
>> ---
>>   include/net/cfg80211.h       |  4 ++++
>>   include/uapi/linux/nl80211.h | 14 ++++++++++++++
>>   net/wireless/nl80211.c       |  4 +++-
>>   net/wireless/sme.c           |  1 +
>>   4 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>> index 115f6fc5a34d..f9f4fde2dc09 100644
>> --- a/include/net/cfg80211.h
>> +++ b/include/net/cfg80211.h
>> @@ -5384,6 +5384,9 @@ cfg80211_connect_timeout(struct net_device *dev,
>> const u8 *bssid,
>>    * @req_ie_len: association request IEs length
>>    * @resp_ie: association response IEs (may be %NULL)
>>    * @resp_ie_len: assoc response IEs length
>> + * @authorized: true if the 802.1X authentication was done by the
>> driver or is
>> + *    not needed (e.g., when Fast Transition protocol was used), false
>> + *    otherwise. Ignored for networks that don't use 802.1X
>> authentication.
> 
> It is not ignored in this patch so it is expected user-space behavior
> you are describing, which is not really needed here in cfg80211 driver api.
> 
>>    */
>>   struct cfg80211_roam_info {
>>       struct ieee80211_channel *channel;
>> @@ -5393,6 +5396,7 @@ struct cfg80211_roam_info {
>>       size_t req_ie_len;
>>       const u8 *resp_ie;
>>       size_t resp_ie_len;
>> +    bool authorized;
>>   };
>>     /**
>> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
>> index 6095a6c4c412..7bdbce7c4147 100644
>> --- a/include/uapi/linux/nl80211.h
>> +++ b/include/uapi/linux/nl80211.h
>> @@ -546,6 +546,12 @@
>>    *    well to remain backwards compatible.
>>    * @NL80211_CMD_ROAM: request that the card roam (currently not
>> implemented),
> 
> Do we want to keep this comment about the request scenario. Is it likely
> implemented soon/ever?
> 
>>    *    sent as an event when the card/driver roamed by itself.
>> + *    When used as an event, and the driver roamed in a network that
>> requires
>> + *    802.1X authentication, %NL80211_ATTR_CONNECTION_AUTHORIZED
>> should be set
>> + *    if the 802.1X authentication was done by the driver or if
>> roaming was
>> + *    done using Fast Transition protocol (in which case 802.1X
>> authentication
>> + *    is not needed). If %NL80211_ATTR_CONNECTION_AUTHORIZED is not set,
>> + *    user space is responsible for the 802.1X authentication.
> 
> Would you consider using NL80211_ATTR_PORT_AUTHORIZED instead referring
> to the 802.1X port entities.

In wpa_supplicant the function mlme_event_connect() is used to process
NL80211_CMD_CONNECT and NL80211_CMD_ROAM events. The latter is actually
used for processing QCA vendor specific event, which passes a nlattr
called authorized to the function. It is typed as u8:

	if (authorized && nla_get_u8(authorized)) {
		event.assoc_info.authorized = 1;
		wpa_printf(MSG_DEBUG, "nl80211: connection authorized");
	}

Not really a good argument, but choosing the same type for the new
attribute would make supporting it relatively easy.

Regards,
Arend
Johannes Berg April 28, 2017, 9:02 p.m. UTC | #3
On Wed, 2017-04-26 at 12:05 +0200, Arend van Spriel wrote:
> 
> > the mobility domain does not require new 802.1X authentication, but
> > roaming to another mobility domain does.
> 
> Not sure about the terminology here. Is "mobility domain" the same
> as  "ESS" which stands for extended service set as definced in
> 802.11  standard. If so, I would prefer use of that term here.

No. "Mobility domain" was defined in 802.11r, it's unrelated to ESS.

> > + * @authorized: true if the 802.1X authentication was done by the
> > driver or is
> > + *	not needed (e.g., when Fast Transition protocol was
> > used), false
> > + *	otherwise. Ignored for networks that don't use 802.1X
> > authentication.
> 
> It is not ignored in this patch so it is expected user-space
> behavior you are describing, which is not really needed here in
> cfg80211 driver api.

Still kinda makes sense though to give that hint to the driver authors,
don't you think? Though it should be pretty clear ...

> >    *	well to remain backwards compatible.
> >    * @NL80211_CMD_ROAM: request that the card roam (currently not
> > implemented),
> 
> Do we want to keep this comment about the request scenario. Is it
> likely implemented soon/ever?

Heh, good question. Probably not, but it's kinda an unrelated cleanup,
no?

> >    *	sent as an event when the card/driver roamed by itself.
> > + *	When used as an event, and the driver roamed in a
> > network that requires
> > + *	802.1X authentication,
> > %NL80211_ATTR_CONNECTION_AUTHORIZED should be set
> > + *	if the 802.1X authentication was done by the driver or
> > if roaming was
> > + *	done using Fast Transition protocol (in which case
> > 802.1X authentication
> > + *	is not needed). If %NL80211_ATTR_CONNECTION_AUTHORIZED
> > is not set,
> > + *	user space is responsible for the 802.1X authentication.
> 
> Would you consider using NL80211_ATTR_PORT_AUTHORIZED instead
> referring to the 802.1X port entities.

I guess that makes sense, yeah.

johannes
Arend van Spriel May 1, 2017, 9:40 a.m. UTC | #4
On 4/28/2017 11:02 PM, Johannes Berg wrote:
> On Wed, 2017-04-26 at 12:05 +0200, Arend van Spriel wrote:
>>
>>> the mobility domain does not require new 802.1X authentication, but
>>> roaming to another mobility domain does.
>>
>> Not sure about the terminology here. Is "mobility domain" the same
>> as  "ESS" which stands for extended service set as definced in
>> 802.11  standard. If so, I would prefer use of that term here.
> 
> No. "Mobility domain" was defined in 802.11r, it's unrelated to ESS.

Thanks for the reference. So a "Mobility domain" is a group of BSSes 
within the same ESS. Noted.

>>> + * @authorized: true if the 802.1X authentication was done by the
>>> driver or is
>>> + *	not needed (e.g., when Fast Transition protocol was
>>> used), false
>>> + *	otherwise. Ignored for networks that don't use 802.1X
>>> authentication.
>>
>> It is not ignored in this patch so it is expected user-space
>> behavior you are describing, which is not really needed here in
>> cfg80211 driver api.
> 
> Still kinda makes sense though to give that hint to the driver authors,
> don't you think? Though it should be pretty clear ...

I tend to look further than the cfg80211 API ;-) Agree that is good to 
have it within the scope of the cfg80211 driver API.

>>>     *	well to remain backwards compatible.
>>>     * @NL80211_CMD_ROAM: request that the card roam (currently not
>>> implemented),
>>
>> Do we want to keep this comment about the request scenario. Is it
>> likely implemented soon/ever?
> 
> Heh, good question. Probably not, but it's kinda an unrelated cleanup,
> no?

True. It just seems a good time to do it.

>>>     *	sent as an event when the card/driver roamed by itself.
>>> + *	When used as an event, and the driver roamed in a
>>> network that requires
>>> + *	802.1X authentication,
>>> %NL80211_ATTR_CONNECTION_AUTHORIZED should be set
>>> + *	if the 802.1X authentication was done by the driver or
>>> if roaming was
>>> + *	done using Fast Transition protocol (in which case
>>> 802.1X authentication
>>> + *	is not needed). If %NL80211_ATTR_CONNECTION_AUTHORIZED
>>> is not set,
>>> + *	user space is responsible for the 802.1X authentication.
>>
>> Would you consider using NL80211_ATTR_PORT_AUTHORIZED instead
>> referring to the 802.1X port entities.
> 
> I guess that makes sense, yeah.

So I will include this patch in my patchset for PSK/1X offloading taken 
above into account.

Regards,
Arend
Johannes Berg May 2, 2017, 6:59 a.m. UTC | #5
On Mon, 2017-05-01 at 11:40 +0200, Arend van Spriel wrote:
> 
> So I will include this patch in my patchset for PSK/1X offloading
> taken above into account.

Great, thanks!

johannes
diff mbox

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 115f6fc5a34d..f9f4fde2dc09 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5384,6 +5384,9 @@  cfg80211_connect_timeout(struct net_device *dev, const u8 *bssid,
  * @req_ie_len: association request IEs length
  * @resp_ie: association response IEs (may be %NULL)
  * @resp_ie_len: assoc response IEs length
+ * @authorized: true if the 802.1X authentication was done by the driver or is
+ *	not needed (e.g., when Fast Transition protocol was used), false
+ *	otherwise. Ignored for networks that don't use 802.1X authentication.
  */
 struct cfg80211_roam_info {
 	struct ieee80211_channel *channel;
@@ -5393,6 +5396,7 @@  struct cfg80211_roam_info {
 	size_t req_ie_len;
 	const u8 *resp_ie;
 	size_t resp_ie_len;
+	bool authorized;
 };
 
 /**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 6095a6c4c412..7bdbce7c4147 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -546,6 +546,12 @@ 
  *	well to remain backwards compatible.
  * @NL80211_CMD_ROAM: request that the card roam (currently not implemented),
  *	sent as an event when the card/driver roamed by itself.
+ *	When used as an event, and the driver roamed in a network that requires
+ *	802.1X authentication, %NL80211_ATTR_CONNECTION_AUTHORIZED should be set
+ *	if the 802.1X authentication was done by the driver or if roaming was
+ *	done using Fast Transition protocol (in which case 802.1X authentication
+ *	is not needed). If %NL80211_ATTR_CONNECTION_AUTHORIZED is not set,
+ *	user space is responsible for the 802.1X authentication.
  * @NL80211_CMD_DISCONNECT: drop a given connection; also used to notify
  *	userspace that a connection was dropped by the AP or due to other
  *	reasons, for this the %NL80211_ATTR_DISCONNECTED_BY_AP and
@@ -2081,6 +2087,12 @@  enum nl80211_commands {
  * @NL80211_ATTR_PMK: PMK for the PMKSA identified by %NL80211_ATTR_PMKID.
  *	This is used with @NL80211_CMD_SET_PMKSA.
  *
+ * @NL80211_ATTR_CONNECTION_AUTHORIZED: A flag attribute used with
+ *	%NL80211_CMD_ROAM to indicate that 802.1X authentication was done by the
+ *	driver or is not needed (because roaming used the Fast Transition
+ *	protocol). Only valid for roaming in networks that require 802.1X
+ *	authentication.
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -2500,6 +2512,8 @@  enum nl80211_attrs {
 
 	NL80211_ATTR_PMK,
 
+	NL80211_ATTR_CONNECTION_AUTHORIZED,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index ab9036e2d622..f3bf20584222 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -13621,7 +13621,9 @@  void nl80211_send_roamed(struct cfg80211_registered_device *rdev,
 		     info->req_ie)) ||
 	    (info->resp_ie &&
 	     nla_put(msg, NL80211_ATTR_RESP_IE, info->resp_ie_len,
-		     info->resp_ie)))
+		     info->resp_ie)) ||
+	    (info->authorized &&
+	     nla_put_flag(msg, NL80211_ATTR_CONNECTION_AUTHORIZED)))
 		goto nla_put_failure;
 
 	genlmsg_end(msg, hdr);
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 532a0007ce82..0a49b88070d0 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -960,6 +960,7 @@  void cfg80211_roamed(struct net_device *dev, struct cfg80211_roam_info *info,
 	ev->rm.resp_ie_len = info->resp_ie_len;
 	memcpy((void *)ev->rm.resp_ie, info->resp_ie, info->resp_ie_len);
 	ev->rm.bss = info->bss;
+	ev->rm.authorized = info->authorized;
 
 	spin_lock_irqsave(&wdev->event_lock, flags);
 	list_add_tail(&ev->list, &wdev->event_list);