[PATCHv3 1/2] cfg80211: add the new IBSS_STA event

[PATCHv3 1/2] cfg80211: add the new IBSS_STA event

[Antonio Quartulli]

In IBSS mode it could be the case that a station being added
is not ready for starting higher level routines (e.g. key
exchange when IBSS/RSN is enabled). In particular, this can
happen when we are waiting for the AUTH response message.

This IBSS_STA event is triggered after the NEW_STA one and
only when the upper layer is allowed to assume that the new
peer is ready

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---

v2:

 1/2:
 * untouched
 2/2:
 * if userspace registered for auth frames, the new station is immediately
   authenticated and the new IBSS_STA event is triggered.

v3:

 1/2:
 * untouched
 2/2:
 * save last_auth time in the proper place (when mac80211 sends the AUTH
   request)


 include/net/cfg80211.h       |  8 ++++++++
 include/uapi/linux/nl80211.h |  2 ++
 net/wireless/mlme.c          |  9 +++++++++
 net/wireless/nl80211.c       | 33 +++++++++++++++++++++++++++++++++
 net/wireless/nl80211.h       |  3 +++
 5 files changed, 55 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 8e6a6b7..3266c03 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -3497,6 +3497,14 @@ void cfg80211_new_sta(struct net_device *dev, const u8 *mac_addr,
 		      struct station_info *sinfo, gfp_t gfp);
 
 /**
+ * cfg80211_ibss_sta - notify userspace that the ibss station is ready
+ * @dev: the netdev
+ * @mac_addr: the station's address
+ * @gfp: allocation flags
+ */
+void cfg80211_ibss_sta(struct net_device *dev, const u8 *mac_addr, gfp_t gfp);
+
+/**
  * cfg80211_del_sta - notify userspace about deletion of a station
  *
  * @dev: the netdev
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index e3e19f8..7e0911e 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -736,6 +736,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_SET_MCAST_RATE,
 
+	NL80211_CMD_IBSS_STATION,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 5e8123e..f9493fc 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -612,6 +612,15 @@ void cfg80211_new_sta(struct net_device *dev, const u8 *mac_addr,
 }
 EXPORT_SYMBOL(cfg80211_new_sta);
 
+void cfg80211_ibss_sta(struct net_device *dev, const u8 *mac_addr, gfp_t gfp)
+{
+	struct wiphy *wiphy = dev->ieee80211_ptr->wiphy;
+	struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
+
+	nl80211_send_ibss_sta_event(rdev, dev, mac_addr, gfp);
+}
+EXPORT_SYMBOL(cfg80211_ibss_sta);
+
 void cfg80211_del_sta(struct net_device *dev, const u8 *mac_addr, gfp_t gfp)
 {
 	struct wiphy *wiphy = dev->ieee80211_ptr->wiphy;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index f45706a..2d27911 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -8564,6 +8564,39 @@ void nl80211_send_sta_event(struct cfg80211_registered_device *rdev,
 				nl80211_mlme_mcgrp.id, gfp);
 }
 
+void nl80211_send_ibss_sta_event(struct cfg80211_registered_device *rdev,
+				 struct net_device *dev, const u8 *mac_addr,
+				 gfp_t gfp)
+{
+	struct sk_buff *msg;
+	void *hdr;
+
+	msg = nlmsg_new(NLMSG_GOODSIZE, gfp);
+	if (!msg)
+		return;
+
+	hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_IBSS_STATION);
+	if (!hdr) {
+		nlmsg_free(msg);
+		return;
+	}
+
+	if (nla_put_u32(msg, NL80211_ATTR_IFINDEX, dev->ifindex) ||
+	    nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, mac_addr))
+		goto nla_put_failure;
+
+	genlmsg_end(msg, hdr);
+
+	genlmsg_multicast_netns(wiphy_net(&rdev->wiphy), msg, 0,
+				nl80211_mlme_mcgrp.id, gfp);
+	return;
+
+ nla_put_failure:
+	genlmsg_cancel(msg, hdr);
+	nlmsg_free(msg);
+}
+
+
 void nl80211_send_sta_del_event(struct cfg80211_registered_device *rdev,
 				struct net_device *dev, const u8 *mac_addr,
 				gfp_t gfp)
diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h
index 2acba84..429f5d6 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -85,6 +85,9 @@ void nl80211_send_remain_on_channel_cancel(
 void nl80211_send_sta_event(struct cfg80211_registered_device *rdev,
 			    struct net_device *dev, const u8 *mac_addr,
 			    struct station_info *sinfo, gfp_t gfp);
+void nl80211_send_ibss_sta_event(struct cfg80211_registered_device *rdev,
+				 struct net_device *dev, const u8 *mac_addr,
+				 gfp_t gfp);
 void nl80211_send_sta_del_event(struct cfg80211_registered_device *rdev,
 				struct net_device *dev, const u8 *mac_addr,
 				gfp_t gfp);
-- 
1.8.0

[PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Antonio Quartulli]

To prevent race conditions between kernel state and
user-space application knowledge, it is better to wait for
a new peer to be completely authenticated before telling the
userspace that it is ready to start authorization
(IBSS/RSN).

Use the IBSS_STA event to tell userspace when a station is
authenticated and ready.

It could still be the case that the other node joining the
AD-HOC cell does not implement AUTH messages exchange,
therefore a fallback mechanism will authenticate the peer
after a timeout set for the purpose expires

Signed-off-by: Antonio Quartulli <antonio@open-mesh.com>
---
 net/mac80211/ibss.c     | 103 +++++++++++++++++++++++++++++++++++++++---------
 net/mac80211/sta_info.h |   2 +
 2 files changed, 87 insertions(+), 18 deletions(-)

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 700d0ed..91956e3 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -30,6 +30,7 @@
 
 #define IEEE80211_IBSS_MERGE_INTERVAL (30 * HZ)
 #define IEEE80211_IBSS_INACTIVITY_LIMIT (60 * HZ)
+#define IEEE80211_IBSS_AUTH_TIMEOUT (HZ / 2)
 
 #define IEEE80211_IBSS_MAX_STA_ENTRIES 128
 
@@ -273,8 +274,7 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 				  false);
 }
 
-static struct sta_info *ieee80211_ibss_finish_sta(struct sta_info *sta,
-						  bool auth)
+static struct sta_info *ieee80211_ibss_finish_sta(struct sta_info *sta)
 	__acquires(RCU)
 {
 	struct ieee80211_sub_if_data *sdata = sta->sdata;
@@ -284,32 +284,33 @@ static struct sta_info *ieee80211_ibss_finish_sta(struct sta_info *sta,
 
 	ibss_dbg(sdata, "Adding new IBSS station %pM\n", addr);
 
-	sta_info_pre_move_state(sta, IEEE80211_STA_AUTH);
-	sta_info_pre_move_state(sta, IEEE80211_STA_ASSOC);
-	/* authorize the station only if the network is not RSN protected. If
-	 * not wait for the userspace to authorize it */
-	if (!sta->sdata->u.ibss.control_port)
-		sta_info_pre_move_state(sta, IEEE80211_STA_AUTHORIZED);
-
 	rate_control_rate_init(sta);
 
 	/* If it fails, maybe we raced another insertion? */
 	if (sta_info_insert_rcu(sta))
 		return sta_info_get(sdata, addr);
-	if (auth && !sdata->u.ibss.auth_frame_registrations) {
+	if (!sdata->u.ibss.auth_frame_registrations) {
 		ibss_dbg(sdata,
 			 "TX Auth SA=%pM DA=%pM BSSID=%pM (auth_transaction=1)\n",
 			 sdata->vif.addr, addr, sdata->u.ibss.bssid);
 		ieee80211_send_auth(sdata, 1, WLAN_AUTH_OPEN, 0, NULL, 0,
 				    addr, sdata->u.ibss.bssid, NULL, 0, 0);
+		/* store when the AUTH request has been sent so that this
+		 * station can eventually be authorise with the fall back
+		 * mechanism
+		 */
+		sta->last_auth = jiffies;
+	} else {
+		ieee80211_ibss_auth_sta(sta);
 	}
+
 	return sta;
 }
 
 static struct sta_info *
 ieee80211_ibss_add_sta(struct ieee80211_sub_if_data *sdata,
 		       const u8 *bssid, const u8 *addr,
-		       u32 supp_rates, bool auth)
+		       u32 supp_rates)
 	__acquires(RCU)
 {
 	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
@@ -358,7 +359,23 @@ ieee80211_ibss_add_sta(struct ieee80211_sub_if_data *sdata,
 	sta->sta.supp_rates[band] = supp_rates |
 			ieee80211_mandatory_rates(local, band);
 
-	return ieee80211_ibss_finish_sta(sta, auth);
+	return ieee80211_ibss_finish_sta(sta);
+}
+
+static void ieee80211_ibss_auth_sta(struct sta_info *sta)
+{
+	if (sta->sta_state > IEEE80211_STA_NONE)
+		return;
+
+	sta_info_move_state(sta, IEEE80211_STA_AUTH);
+	sta_info_move_state(sta, IEEE80211_STA_ASSOC);
+	/* authorize the station only if the network is not RSN protected. If
+	 * not wait for the userspace to authorize it
+	 */
+	if (!sta->sdata->u.ibss.control_port)
+		sta_info_move_state(sta, IEEE80211_STA_AUTHORIZED);
+
+	cfg80211_ibss_sta(sta->sdata->dev, sta->sta.addr, GFP_KERNEL);
 }
 
 static void ieee80211_rx_mgmt_deauth_ibss(struct ieee80211_sub_if_data *sdata,
@@ -395,13 +412,36 @@ static void ieee80211_rx_mgmt_auth_ibss(struct ieee80211_sub_if_data *sdata,
 		 "RX Auth SA=%pM DA=%pM BSSID=%pM (auth_transaction=%d)\n",
 		 mgmt->sa, mgmt->da, mgmt->bssid, auth_transaction);
 
-	if (auth_alg != WLAN_AUTH_OPEN || auth_transaction != 1)
+	if (auth_alg != WLAN_AUTH_OPEN)
 		return;
 
-	sta_info_destroy_addr(sdata, mgmt->sa);
-	sta = ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, 0, false);
+	rcu_read_lock();
+	sta = sta_info_get(sdata, mgmt->sa);
 	rcu_read_unlock();
 
+	if (auth_transaction == 2) {
+		ibss_dbg(sdata, "Authenticating STA %pM\n", sta->sta.addr);
+		ieee80211_ibss_auth_sta(sta);
+		return;
+	}
+
+	/* drop bogus auth frames (auth_transaction can be only 1 or 2) */
+	if (auth_transaction != 1)
+		return;
+
+	if (sta && sdata->u.ibss.control_port &&
+	    sta->sta_state == IEEE80211_STA_AUTHORIZED) {
+		ibss_dbg(sdata, "Resetting STA %pM state for IBSS Encryption\n",
+			 sta->sta.addr);
+		sta_info_destroy_addr(sdata, mgmt->sa);
+		sta = NULL;
+	}
+
+	if (!sta) {
+		sta = ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, 0);
+		rcu_read_unlock();
+	}
+
 	/*
 	 * if we have any problem in allocating the new station, we reply with a
 	 * DEAUTH frame to tell the other end that we had a problem
@@ -420,6 +460,8 @@ static void ieee80211_rx_mgmt_auth_ibss(struct ieee80211_sub_if_data *sdata,
 	 * However, try to reply to authentication attempts if someone
 	 * has actually implemented this.
 	 */
+	ibss_dbg(sdata, "TX Auth SA=%pM DA=%pM BSSID=%pM (auth_transaction=2)\n",
+		 sdata->vif.addr, mgmt->sa, sdata->u.ibss.bssid);
 	ieee80211_send_auth(sdata, 2, WLAN_AUTH_OPEN, 0, NULL, 0,
 			    mgmt->sa, sdata->u.ibss.bssid, NULL, 0, 0);
 }
@@ -481,7 +523,7 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
 			} else {
 				rcu_read_unlock();
 				sta = ieee80211_ibss_add_sta(sdata, mgmt->bssid,
-						mgmt->sa, supp_rates, true);
+						mgmt->sa, supp_rates);
 			}
 		}
 
@@ -592,7 +634,7 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
 		ieee80211_sta_join_ibss(sdata, bss);
 		supp_rates = ieee80211_sta_get_rates(local, elems, band, NULL);
 		ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
-				       supp_rates, true);
+				       supp_rates);
 		rcu_read_unlock();
 	}
 
@@ -675,6 +717,29 @@ static int ieee80211_sta_active_ibss(struct ieee80211_sub_if_data *sdata)
 	return active;
 }
 
+static void ieee80211_ibss_auth_expire(struct ieee80211_sub_if_data *sdata)
+{
+	struct ieee80211_local *local = sdata->local;
+	struct sta_info *sta;
+
+	list_for_each_entry(sta, &local->sta_list, list) {
+		if (sdata != sta->sdata)
+			continue;
+
+		if (sta->sta_state > IEEE80211_STA_NONE)
+			continue;
+
+		if (time_after(jiffies,
+			       sta->last_auth + IEEE80211_IBSS_AUTH_TIMEOUT)) {
+			ibss_dbg(sdata,
+				 "Authenticating IBSS STA %pM by timeout\n",
+				 sta->sta.addr);
+			ieee80211_ibss_auth_sta(sta);
+		}
+	}
+}
+
+
 /*
  * This function is called with state == IEEE80211_IBSS_MLME_JOINED
  */
@@ -688,6 +753,8 @@ static void ieee80211_sta_merge_ibss(struct ieee80211_sub_if_data *sdata)
 	mod_timer(&ifibss->timer,
 		  round_jiffies(jiffies + IEEE80211_IBSS_MERGE_INTERVAL));
 
+	ieee80211_ibss_auth_expire(sdata);
+
 	ieee80211_sta_expire(sdata, IEEE80211_IBSS_INACTIVITY_LIMIT);
 
 	if (time_before(jiffies, ifibss->last_scan_completed +
@@ -976,7 +1043,7 @@ void ieee80211_ibss_work(struct ieee80211_sub_if_data *sdata)
 		list_del(&sta->list);
 		spin_unlock_bh(&ifibss->incomplete_lock);
 
-		ieee80211_ibss_finish_sta(sta, true);
+		ieee80211_ibss_finish_sta(sta);
 		rcu_read_unlock();
 		spin_lock_bh(&ifibss->incomplete_lock);
 	}
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 1489bca..3c150a4 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -244,6 +244,7 @@ struct sta_ampdu_mlme {
  * @rx_bytes: Number of bytes received from this STA
  * @wep_weak_iv_count: number of weak WEP IVs received from this station
  * @last_rx: time (in jiffies) when last frame was received from this STA
+ * @last_auth: jiffies of last auth packet with seq = 1
  * @last_connected: time (in seconds) when a station got connected
  * @num_duplicates: number of duplicate frames received from this STA
  * @rx_fragments: number of received MPDUs
@@ -324,6 +325,7 @@ struct sta_info {
 	unsigned long rx_packets, rx_bytes;
 	unsigned long wep_weak_iv_count;
 	unsigned long last_rx;
+	unsigned long last_auth;
 	long last_connected;
 	unsigned long num_duplicates;
 	unsigned long rx_fragments;
-- 
1.8.0

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Johannes Berg]

Ok so I've let these patches languish for far too long ...



> +	} else {
> +		ieee80211_ibss_auth_sta(sta);

How does this compile? I see no static forward declaration (which I
wouldn't like anyway) and the function is defined only later?

Anyway this part is really confusing. If userspace is handling auth
frames, should the kernel really mark the station as authenticated? What
then is the point in handling auth frames in userspace??

Will?

Any chance we could converge on a single implementation here?

> +static void ieee80211_ibss_auth_sta(struct sta_info *sta)
> +{
> +	if (sta->sta_state > IEEE80211_STA_NONE)
> +		return;
> +
> +	sta_info_move_state(sta, IEEE80211_STA_AUTH);
> +	sta_info_move_state(sta, IEEE80211_STA_ASSOC);
> +	/* authorize the station only if the network is not RSN protected. If
> +	 * not wait for the userspace to authorize it
> +	 */

technically, control_port != RSN protected, but I guess for IBSS ...
who's going to do WAPI? ;-)

> +	cfg80211_ibss_sta(sta->sdata->dev, sta->sta.addr, GFP_KERNEL);

I'm not really convinced that this event is the right thing to do.

If userspace is handing the auth frame, it already has the event right
there, no? It can even reply, even if for some reason it can't send a
negative reply because the kernel has already marked the station as
authenticated (see above).

If there's no userspace, then RSN can't work anyway, so maybe this isn't
really needed?

Also, a more generic event would make more sense I think?

> +	ieee80211_ibss_auth_expire(sdata);

It seems you should have something to trigger the timer too?

johannes

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 3441 bytes --]

Hi Johannes,

On Fri, Dec 28, 2012 at 03:51:08 +0100, Johannes Berg wrote:
> Anyway this part is really confusing. If userspace is handling auth
> frames, should the kernel really mark the station as authenticated? What
> then is the point in handling auth frames in userspace??
> 
> Will?

Actually I don't know. Maybe Will can help us in understanding how to handle this point.
I simply kept the same behaviour: before this (my) patch a station was marked as
AUTHenticated in by the kernel, even if userspace registered for AUTH frames,
and the same I'm doing here.


> 
> Any chance we could converge on a single implementation here?
> 

Maybe yes :)

I think that leaving the station not AUTHenticated and let userspace do so would
be the best approach..but then we need a way to enable userspace to do it :)


> > +static void ieee80211_ibss_auth_sta(struct sta_info *sta)
> > +{
> > +	if (sta->sta_state > IEEE80211_STA_NONE)
> > +		return;
> > +
> > +	sta_info_move_state(sta, IEEE80211_STA_AUTH);
> > +	sta_info_move_state(sta, IEEE80211_STA_ASSOC);
> > +	/* authorize the station only if the network is not RSN protected. If
> > +	 * not wait for the userspace to authorize it
> > +	 */
> 
> technically, control_port != RSN protected, but I guess for IBSS ...
> who's going to do WAPI? ;-)

This is a behaviour I introduced some time ago: 

1153         sdata->u.ibss.control_port = params->control_port;

which is set based on NL80211_ATTR_CONTROL_PORT and at the moment, in IBSS, I
think it is only set by wpa_supplicant when using IBSS/RSN..you told me to use
control_port instead of creating another member..but maybe it is general and
could be used for something else?

> 
> > +	cfg80211_ibss_sta(sta->sdata->dev, sta->sta.addr, GFP_KERNEL);
> 
> I'm not really convinced that this event is the right thing to do.
> 
> If userspace is handing the auth frame, it already has the event right
> there, no? It can even reply, even if for some reason it can't send a
> negative reply because the kernel has already marked the station as
> authenticated (see above).
> 

Here I am missing something. To the best of knowledge, wpa_supplicant (which is
the only userspace tool providing IBSS/RSN and it is not handling AUTH frames.
Therefore it will not have any event for authentication..

> If there's no userspace, then RSN can't work anyway, so maybe this isn't
> really needed?

what we could do is to "move" the NEW_STA event and use it in this point. But I
preferred to create a new event type in order to distinguish between the case of
detecting a new station from the case of having a new station ready for key
exchange.

> 
> Also, a more generic event would make more sense I think?
> 

You mean a generic event to be triggered on state change? Yes sure, but at the
moment it was much more important to reshape the interaction between US and KS
when using IBSS/RSN.

> > +	ieee80211_ibss_auth_expire(sdata);
> 
> It seems you should have something to trigger the timer too?
> 

uhm? I don't understand. This function is invoked in ieee80211_sta_merge_ibss()
which should be periodically called (if I got it correctly...the timer stuff in
ibss is not really clear to me)

Thank you very much for your feedbacks!
Happy new year :)

Cheers,


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Nicolas Cavallari]

On 02/01/2013 07:32, Antonio Quartulli wrote:
> Hi Johannes,
> 
> On Fri, Dec 28, 2012 at 03:51:08 +0100, Johannes Berg wrote:
>> Anyway this part is really confusing. If userspace is handling auth
>> frames, should the kernel really mark the station as authenticated? What
>> then is the point in handling auth frames in userspace??

In IBSS mode, the "auth" state is essentially a no-op with the current code, just saying.

I do open system authentication in userspace, partly to fix these race conditions in a
clean way, and to have more control over it in a more convenient way. That way, it is
possible to hack things like retrying authentication at the user will if it failed the
first time.
I can also handle stations that do not answer auth frames much easier, such as skipping
open system authentication if the other station engage actually tries to authenticate with us.

>> Any chance we could converge on a single implementation here?
>>
> 
> Maybe yes :)
> 
> I think that leaving the station not AUTHenticated and let userspace do so would
> be the best approach..but then we need a way to enable userspace to do it :)

What would happen to old userspace that expect that to be done in the kernel ?

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]

On Wed, Jan 02, 2013 at 10:40:55AM +0100, Nicolas Cavallari wrote:
> On 02/01/2013 07:32, Antonio Quartulli wrote:
> > Hi Johannes,
> > 
> > On Fri, Dec 28, 2012 at 03:51:08 +0100, Johannes Berg wrote:
> >> Anyway this part is really confusing. If userspace is handling auth
> >> frames, should the kernel really mark the station as authenticated? What
> >> then is the point in handling auth frames in userspace??
> 
> In IBSS mode, the "auth" state is essentially a no-op with the current code, just saying.
> 
> I do open system authentication in userspace, partly to fix these race conditions in a
> clean way, and to have more control over it in a more convenient way. That way, it is
> possible to hack things like retrying authentication at the user will if it failed the
> first time.
> I can also handle stations that do not answer auth frames much easier, such as skipping
> open system authentication if the other station engage actually tries to authenticate with us.
> 
> >> Any chance we could converge on a single implementation here?
> >>
> > 
> > Maybe yes :)
> > 
> > I think that leaving the station not AUTHenticated and let userspace do so would
> > be the best approach..but then we need a way to enable userspace to do it :)
> 
> What would happen to old userspace that expect that to be done in the kernel ?

if the userspace did not register for auth frames, then authentication will be
handled in the kernel (as I'm doing/changing now), while if the userspace
registered for such frames, then the kernel will assume that the authentication
will be handled by the userspace somehow and will not deal with it (but again,
in this way we need a command to let userspace set the AUTHenticated flag on a
station).

Cheers,


> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Nicolas Cavallari]

On 07/01/2013 12:40, Antonio Quartulli wrote:
> On Wed, Jan 02, 2013 at 10:40:55AM +0100, Nicolas Cavallari wrote:
>> On 02/01/2013 07:32, Antonio Quartulli wrote:
>>> On Fri, Dec 28, 2012 at 03:51:08 +0100, Johannes Berg wrote:
>>>> Any chance we could converge on a single implementation here?
>>>
>>> Maybe yes :)
>>>
>>> I think that leaving the station not AUTHenticated and let userspace do so would
>>> be the best approach..but then we need a way to enable userspace to do it :)
>>
>> What would happen to old userspace that expect that to be done in the kernel ?
> 
> if the userspace did not register for auth frames, then authentication will be
> handled in the kernel (as I'm doing/changing now), while if the userspace
> registered for such frames, then the kernel will assume that the authentication
> will be handled by the userspace somehow and will not deal with it (but again,
> in this way we need a command to let userspace set the AUTHenticated flag on a
> station).

That means two implementations, not one.

And even with this current patch, old wpasupplicant starts 4 way handshake as soon as the
NEW_STA event is received, even if the station is not authenticated by the kernel. I
expect that some EAPOL frames will be dropped as a result.

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Johannes Berg]

On Mon, 2013-01-07 at 14:16 +0100, Nicolas Cavallari wrote:

> > if the userspace did not register for auth frames, then authentication will be
> > handled in the kernel (as I'm doing/changing now), while if the userspace
> > registered for such frames, then the kernel will assume that the authentication
> > will be handled by the userspace somehow and will not deal with it (but again,
> > in this way we need a command to let userspace set the AUTHenticated flag on a
> > station).
> 
> That means two implementations, not one.
> 
> And even with this current patch, old wpasupplicant starts 4 way handshake as soon as the
> NEW_STA event is received, even if the station is not authenticated by the kernel. I
> expect that some EAPOL frames will be dropped as a result.

That's all very confusing to me.

Today, we have basically two ways of operating, userspace-authorization
or open network. Neither of them requires AUTH frames, except SAE which
uses them in userspace.

In either case, there are a few ways a station can be added:
1) when mac80211 receives a data frame from a station matching the BSSID
2) when mac80211 receives a proper open network authentication frame
   from the station (except this is bypassed in SAE, since then
   userspace gets all authentication frames instead of mac80211)
3) when mac80211 receives a beacon or probe response from a station

In all of these cases, the station is marked as authenticated (and
associated for internal purposes, as this doesn't really exist in IBSS.)
Authorization can be deferred to userspace, or done inline, depending on
the "control port" setting.


I think part of the reason this particular patch is so confusing is that
it changes the semantics of when we add another station, based on our
own authenticating with it? That's pretty confusing.

The point, I thought, was to detect when a peer "silently rebooted" and
thus lost all state. But in that case wouldn't we receive an unencrypted
RSN handshake frame from the station, and be able to recover based on
that?

johannes

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Nicolas Cavallari]

On 25/01/2013 23:05, Johannes Berg wrote:
> On Mon, 2013-01-07 at 14:16 +0100, Nicolas Cavallari wrote:
> 
>>> if the userspace did not register for auth frames, then authentication will be
>>> handled in the kernel (as I'm doing/changing now), while if the userspace
>>> registered for such frames, then the kernel will assume that the authentication
>>> will be handled by the userspace somehow and will not deal with it (but again,
>>> in this way we need a command to let userspace set the AUTHenticated flag on a
>>> station).
>>
>> That means two implementations, not one.
>>
>> And even with this current patch, old wpasupplicant starts 4 way handshake as soon as the
>> NEW_STA event is received, even if the station is not authenticated by the kernel. I
>> expect that some EAPOL frames will be dropped as a result.
> 
> That's all very confusing to me.
> 
> Today, we have basically two ways of operating, userspace-authorization
> or open network. Neither of them requires AUTH frames, except SAE which
> uses them in userspace.
> 
> In either case, there are a few ways a station can be added:
> 1) when mac80211 receives a data frame from a station matching the BSSID
> 2) when mac80211 receives a proper open network authentication frame
>    from the station (except this is bypassed in SAE, since then
>    userspace gets all authentication frames instead of mac80211)
> 3) when mac80211 receives a beacon or probe response from a station
> 
> In all of these cases, the station is marked as authenticated (and
> associated for internal purposes, as this doesn't really exist in IBSS.)
> Authorization can be deferred to userspace, or done inline, depending on
> the "control port" setting.

Yes, this is what wpasupplicant assumes: When a station is added, it is
already authenticated. So wpasupplicant does not wait before sending
handshakes.

This causes problem with the current kernel's behavior to send auth
frames, because it can race: wpasupplicant sometimes send it's first 1/4
handshake before the kernel's auth frame. If the peer runs Linux with
the reboot detection, it is utterly confused : the peer's wpasupplicant
receives the 1/4 frame, answers it with 2/4, then the peer's kernel
receives the auth frames, and resets the station, discarding the state
of wpasupplicant, which no longer knows that it has started a handshake.

Meanwhile, on the other host, wpasupplicant happily receives the 2/4
response, and proceed with sending 3/4 frame. Then there is utter
confusion, because the other host does not know what to do with
handshake 3/4 and drops it.

This is what the current patch tries to fix : by making wpasupplicant
waits for the kernel to tell it that it has completed an open system
authentication. This requires patch to both.

(and my take on this, is that we should just handle open system
authentication and reboot detection in userspace (i have code for that),
and revert the kernel to the old state where it would just answers an
open system authentication request if userspace is not handling it)

> I think part of the reason this particular patch is so confusing is that
> it changes the semantics of when we add another station, based on our
> own authenticating with it? That's pretty confusing.

This patch (under the case when userspace is not handling auth frames)
defers authentication until an open system authentication is done or has
timed out, and uses a new event (IBSS_STA) to indicate that a station is
authenticated.

But if you use the patched kernel with the current (unpatched)
wpasupplicant, wpasupplicant will still send handshakes right after a
station is added, even if the station is not authenticated. With enough
bad luck, the open system authentication request or response can be
lost, and wpa_supplicant might complete a handshake before the kernel's
open system authentication timeout, therefore, wpasupplicant would ask
the kernel to authorize a station which is even not authenticated.

(and if you use a patched wpasupplicant with an unpatched kernel, it
will just wait for an event which will never happen)

> The point, I thought, was to detect when a peer "silently rebooted" and
> thus lost all state. But in that case wouldn't we receive an unencrypted
> RSN handshake frame from the station, and be able to recover based on
> that?

wpa_supplicant does not know if an received EAPOL frame was encrypted or
not, so will merely assume that this is just a key renewal, and will
send encrypted responses and keep the old keys until completed.

(and if i remember correctly, Jouni disagreed with this solution a long
time ago, but i can't find the message saying it)

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Johannes Berg]

On Sat, 2013-01-26 at 13:09 +0100, Nicolas Cavallari wrote:

> Yes, this is what wpasupplicant assumes: When a station is added, it is
> already authenticated. So wpasupplicant does not wait before sending
> handshakes.
> 
> This causes problem with the current kernel's behavior to send auth
> frames, because it can race: wpasupplicant sometimes send it's first 1/4
> handshake before the kernel's auth frame. If the peer runs Linux with
> the reboot detection, it is utterly confused : the peer's wpasupplicant
> receives the 1/4 frame, answers it with 2/4, then the peer's kernel
> receives the auth frames, and resets the station, discarding the state
> of wpasupplicant, which no longer knows that it has started a handshake.
> 
> Meanwhile, on the other host, wpasupplicant happily receives the 2/4
> response, and proceed with sending 3/4 frame. Then there is utter
> confusion, because the other host does not know what to do with
> handshake 3/4 and drops it.
> 
> This is what the current patch tries to fix : by making wpasupplicant
> waits for the kernel to tell it that it has completed an open system
> authentication. This requires patch to both.

Ok I guess that explains it a little better.

> (and my take on this, is that we should just handle open system
> authentication and reboot detection in userspace (i have code for that),
> and revert the kernel to the old state where it would just answers an
> open system authentication request if userspace is not handling it)

I think that would make sense. However, to really implement that it
seems that wpa_s should be able to control the in-kernel station
"authenticated" state, not just "authorized" state?

What's the status we have today in the kernel, without the patches, and
what would that "revert" mean?

> This patch (under the case when userspace is not handling auth frames)
> defers authentication until an open system authentication is done or has
> timed out, and uses a new event (IBSS_STA) to indicate that a station is
> authenticated.
> 
> But if you use the patched kernel with the current (unpatched)
> wpasupplicant, wpasupplicant will still send handshakes right after a
> station is added, even if the station is not authenticated. With enough
> bad luck, the open system authentication request or response can be
> lost, and wpa_supplicant might complete a handshake before the kernel's
> open system authentication timeout, therefore, wpasupplicant would ask
> the kernel to authorize a station which is even not authenticated.
> 
> (and if you use a patched wpasupplicant with an unpatched kernel, it
> will just wait for an event which will never happen)

Yeah, both of these cases are really bad, in particular since there's no
way for wpa_s to detect this, and it breaks backward compatibility. I
guess I just made up my mind that I'm not going to apply this.

What changes could we make today to solve this in a way that's
compatible with today's wpa_supplicant (and maybe Will's SAE
implementation, though maybe he wouldn't mind small changes too much)?

johannes

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Nicolas Cavallari]

On 29/01/2013 12:37, Johannes Berg wrote:
> On Sat, 2013-01-26 at 13:09 +0100, Nicolas Cavallari wrote:
>> (and my take on this, is that we should just handle open system
>> authentication and reboot detection in userspace (i have code for that),
>> and revert the kernel to the old state where it would just answers an
>> open system authentication request if userspace is not handling it)
> 
> I think that would make sense. However, to really implement that it
> seems that wpa_s should be able to control the in-kernel station
> "authenticated" state, not just "authorized" state?

Theoritically, yes. However, i don't remember that the "authenticated" state actually
changes anything in IBSS mode. In fact, with the current code, all stations have the
"authenticated" state, whatever the specified parameters and even when the kernel
initiates an open system authentication.

> 
> What's the status we have today in the kernel, without the patches, and
> what would that "revert" mean?

Right now, all IBSS stations have the "authenticated" flag.

If userspace is subscribed to auth frames, userspace have to handle everything, else, if
userspace is not subscribing to auth frames:
- When detecting a new station, the kernel sends an open system auth request, as part of
node reboot detection.
- When the kernel receives an open system authentication request, it destroys the station
and recreates it as part of node reboot detection. If all goes well, it answers it.
- wpasupplicant uses the NEW_STA and DEL_STA events to maintain a list of stations. It
starts RSN handshakes on NEW_STA, and destroy its state on DEL_STA. Eventually, if
handshakes succeeds, wpasupp authorize the stations with SET_STA (i think). much older
wpasupplicant do not support authorizing stations, so the kernel always authorize stations
(but all their unencrypted frames will be dropped until wpasupp configures a PTK after a
successful handshake).

The revert is just my personal taste, but in the case where userspace does not subscribe
to auth frames, i would just make the kernel answer an open system authentication request
if it receives one, and not send any open system auth by itself. That would mean no reboot
detection unless userspace does it. wpasupplicant would maybe need a way to reset the
kernel's sta_info if not already possible.


> What changes could we make today to solve this in a way that's
> compatible with today's wpa_supplicant (and maybe Will's SAE
> implementation, though maybe he wouldn't mind small changes too much)?

Will's SAE will not be affected by any of this, as it's done in userspace by subscribing
to auth frames. If Will need node reboot detection, he has to do it himself.

If we want a working node reboot detection with the current wpasupplicant, we need, in
IBSS mode, to only make the kernel send NEW_STA when a station is authenticated. There's
not much choice here.

If it's acceptable to not have node reboot detection with current wpasupplicant, we could
make a future wpasupplicant add a flag saying it supports the new IBSS_STA event, and the
kernel would only do reboot detection in this case.

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Will Hawkins]

On 01/29/2013 08:59 AM, Nicolas Cavallari wrote:
> On 29/01/2013 12:37, Johannes Berg wrote:
>> On Sat, 2013-01-26 at 13:09 +0100, Nicolas Cavallari wrote:
>>> (and my take on this, is that we should just handle open system
>>> authentication and reboot detection in userspace (i have code for that),
>>> and revert the kernel to the old state where it would just answers an
>>> open system authentication request if userspace is not handling it)
>>
>> I think that would make sense. However, to really implement that it
>> seems that wpa_s should be able to control the in-kernel station
>> "authenticated" state, not just "authorized" state?
> 
> Theoritically, yes. However, i don't remember that the "authenticated" state actually
> changes anything in IBSS mode. In fact, with the current code, all stations have the
> "authenticated" state, whatever the specified parameters and even when the kernel
> initiates an open system authentication.

I agree with this statement. From my reading of the code, it seems that
IBSS nodes are always authenticated. Which is fine, as long as I am not
missing something! :-)

> 
>>
>> What's the status we have today in the kernel, without the patches, and
>> what would that "revert" mean?
> 
> Right now, all IBSS stations have the "authenticated" flag.
> 
> If userspace is subscribed to auth frames, userspace have to handle everything, else, if
> userspace is not subscribing to auth frames:
> - When detecting a new station, the kernel sends an open system auth request, as part of
> node reboot detection.
> - When the kernel receives an open system authentication request, it destroys the station
> and recreates it as part of node reboot detection. If all goes well, it answers it.
> - wpasupplicant uses the NEW_STA and DEL_STA events to maintain a list of stations. It
> starts RSN handshakes on NEW_STA, and destroy its state on DEL_STA. Eventually, if
> handshakes succeeds, wpasupp authorize the stations with SET_STA (i think). much older
> wpasupplicant do not support authorizing stations, so the kernel always authorize stations
> (but all their unencrypted frames will be dropped until wpasupp configures a PTK after a
> successful handshake).
> 
> The revert is just my personal taste, but in the case where userspace does not subscribe
> to auth frames, i would just make the kernel answer an open system authentication request
> if it receives one, and not send any open system auth by itself. That would mean no reboot
> detection unless userspace does it. wpasupplicant would maybe need a way to reset the
> kernel's sta_info if not already possible.
> 
> 
>> What changes could we make today to solve this in a way that's
>> compatible with today's wpa_supplicant (and maybe Will's SAE
>> implementation, though maybe he wouldn't mind small changes too much)?
> 
> Will's SAE will not be affected by any of this, as it's done in userspace by subscribing
> to auth frames. If Will need node reboot detection, he has to do it himself.

Totally agreed! This will not change my code at all. However, having a
"new authenticated station" message (to parallel a message like
NL80211_CMD_NEW_PEER_CANDIDATE from the 802.11s world) would be great
too. That's how I interpreted the IBSS_STA event to work.

The ultimate goal for me is to eventually incorporate SAE support for
IBSS into wpa_s. In other words, I'm following this discussion very
closely and I appreciate everyone's work.

> 
> If we want a working node reboot detection with the current wpasupplicant, we need, in
> IBSS mode, to only make the kernel send NEW_STA when a station is authenticated. There's
> not much choice here.
> 
> If it's acceptable to not have node reboot detection with current wpasupplicant, we could
> make a future wpasupplicant add a flag saying it supports the new IBSS_STA event, and the
> kernel would only do reboot detection in this case.
> 

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Johannes Berg]

On Tue, 2013-01-29 at 14:59 +0100, Nicolas Cavallari wrote:

> > I think that would make sense. However, to really implement that it
> > seems that wpa_s should be able to control the in-kernel station
> > "authenticated" state, not just "authorized" state?
> 
> Theoritically, yes. However, i don't remember that the "authenticated" state actually
> changes anything in IBSS mode. In fact, with the current code, all stations have the
> "authenticated" state, whatever the specified parameters and even when the kernel
> initiates an open system authentication.

Good point, so there's not really a reason to manage this at all.

> > What's the status we have today in the kernel, without the patches, and
> > what would that "revert" mean?
> 
> Right now, all IBSS stations have the "authenticated" flag.
> 
> If userspace is subscribed to auth frames, userspace have to handle everything, else, if
> userspace is not subscribing to auth frames:
> - When detecting a new station, the kernel sends an open system auth request, as part of
> node reboot detection.
> - When the kernel receives an open system authentication request, it destroys the station
> and recreates it as part of node reboot detection. If all goes well, it answers it.
> - wpasupplicant uses the NEW_STA and DEL_STA events to maintain a list of stations. It
> starts RSN handshakes on NEW_STA, and destroy its state on DEL_STA. Eventually, if
> handshakes succeeds, wpasupp authorize the stations with SET_STA (i think). much older
> wpasupplicant do not support authorizing stations, so the kernel always authorize stations
> (but all their unencrypted frames will be dropped until wpasupp configures a PTK after a
> successful handshake).
> 
> The revert is just my personal taste, but in the case where userspace does not subscribe
> to auth frames, i would just make the kernel answer an open system authentication request
> if it receives one, and not send any open system auth by itself. That would mean no reboot
> detection unless userspace does it. wpasupplicant would maybe need a way to reset the
> kernel's sta_info if not already possible.

That seems reasonable. Actually there's no reason for wpa_s to "reset
the [...] sta_info", all it really has to do is set it to unauthorized
if it detects it was rebooted, and then start key negotiation again, I
think?

> > What changes could we make today to solve this in a way that's
> > compatible with today's wpa_supplicant (and maybe Will's SAE
> > implementation, though maybe he wouldn't mind small changes too much)?
> 
> Will's SAE will not be affected by any of this, as it's done in userspace by subscribing
> to auth frames. If Will need node reboot detection, he has to do it himself.

Makes sense.

> If we want a working node reboot detection with the current wpasupplicant, we need, in
> IBSS mode, to only make the kernel send NEW_STA when a station is authenticated. There's
> not much choice here.
> 
> If it's acceptable to not have node reboot detection with current wpasupplicant, we could
> make a future wpasupplicant add a flag saying it supports the new IBSS_STA event, and the
> kernel would only do reboot detection in this case.

I think that's acceptable, but if it requires a wpa_s change anyway we
could just implement reboot detection there instead of adding all these
new events etc.? I.e. rather than having a new supplicant say "OK I will
listen to the right event when handling reboot detection", it could just
use the existing infrastructure and implement it itself?

johannes

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Nicolas Cavallari]

On 31/01/2013 14:34, Johannes Berg wrote:
> That seems reasonable. Actually there's no reason for wpa_s to "reset
> the [...] sta_info", all it really has to do is set it to unauthorized
> if it detects it was rebooted, and then start key negotiation again, I
> think?

I'm not sure why Antonio resetted the sta_info. Not resetting it when a node reboot is
detected from userspace works for me anyway.

>> If we want a working node reboot detection with the current wpasupplicant, we need, in
>> IBSS mode, to only make the kernel send NEW_STA when a station is authenticated. There's
>> not much choice here.
>>
>> If it's acceptable to not have node reboot detection with current wpasupplicant, we could
>> make a future wpasupplicant add a flag saying it supports the new IBSS_STA event, and the
>> kernel would only do reboot detection in this case.
> 
> I think that's acceptable, but if it requires a wpa_s change anyway we
> could just implement reboot detection there instead of adding all these
> new events etc.? I.e. rather than having a new supplicant say "OK I will
> listen to the right event when handling reboot detection", it could just
> use the existing infrastructure and implement it itself?

Well i already have wpa_supplicant patches for that. Might just need to clean that up a
bit so it's at least configurable.

But now i'm reminded that transmitting management frames from userspace requires a
frequency. For wpasupp to be race-free during ibss merges, we should have a way to
transmit management frames to the current bss without specifying a frequency...

(Will once said he uses fixed-channel, so he is not affected)

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Johannes Berg]

On Thu, 2013-01-31 at 15:18 +0100, Nicolas Cavallari wrote:

> > I think that's acceptable, but if it requires a wpa_s change anyway we
> > could just implement reboot detection there instead of adding all these
> > new events etc.? I.e. rather than having a new supplicant say "OK I will
> > listen to the right event when handling reboot detection", it could just
> > use the existing infrastructure and implement it itself?
> 
> Well i already have wpa_supplicant patches for that. Might just need to clean that up a
> bit so it's at least configurable.
> 
> But now i'm reminded that transmitting management frames from userspace requires a
> frequency. For wpasupp to be race-free during ibss merges, we should have a way to
> transmit management frames to the current bss without specifying a frequency...

That seems reasonable.

johannes

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]

Hi all,

let's resurrect this thread :)

On Thu, Jan 31, 2013 at 06:26:13AM -0800, Johannes Berg wrote:
> On Thu, 2013-01-31 at 15:18 +0100, Nicolas Cavallari wrote:
> 
> > > I think that's acceptable, but if it requires a wpa_s change anyway we
> > > could just implement reboot detection there instead of adding all these
> > > new events etc.? I.e. rather than having a new supplicant say "OK I will
> > > listen to the right event when handling reboot detection", it could just
> > > use the existing infrastructure and implement it itself?
> > 
> > Well i already have wpa_supplicant patches for that. Might just need to clean that up a
> > bit so it's at least configurable.
> > 

Would you mind sharing this code? Maybe I can help and offload some work. :)
Anyhow I did not understand how you can detect a node reboot from userspace if, as
you stated, wpa_s is not able to distinguish encrypted from un-encrypted frames.

But why did we trash the idea of simply postponing the moment when NEW_STA is
sent to userspace? It can be sent when the kernel has received the AUTH "Reply"
message and avoid the current race condition.
Actually this would mean to send the NEW_STA at the same moment the kernel with
this patch would send the IBSS_STA.

> > But now i'm reminded that transmitting management frames from userspace requires a
> > frequency. For wpasupp to be race-free during ibss merges, we should have a way to
> > transmit management frames to the current bss without specifying a frequency...
> 
> That seems reasonable.
> 

This is a common problem I faced in other contexts too...


Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Nicolas Cavallari]

On 07/04/2013 23:17, Antonio Quartulli wrote:
> Hi all,
> 
> let's resurrect this thread :)
> 
> On Thu, Jan 31, 2013 at 06:26:13AM -0800, Johannes Berg wrote:
>> On Thu, 2013-01-31 at 15:18 +0100, Nicolas Cavallari wrote:
>>
>>>> I think that's acceptable, but if it requires a wpa_s change anyway we
>>>> could just implement reboot detection there instead of adding all these
>>>> new events etc.? I.e. rather than having a new supplicant say "OK I will
>>>> listen to the right event when handling reboot detection", it could just
>>>> use the existing infrastructure and implement it itself?
>>>
>>> Well i already have wpa_supplicant patches for that. Might just need to clean that up a
>>> bit so it's at least configurable.
>>>
> 
> Would you mind sharing this code?

Well yes, i just need to untangle it with some other local policies ;)

> Maybe I can help and offload some work. :)
> Anyhow I did not understand how you can detect a node reboot from userspace if, as
> you stated, wpa_s is not able to distinguish encrypted from un-encrypted frames.

Well, you do it the same way as you do in the kernel, using auth frames. Except you're not
in the kernel.

>>> But now i'm reminded that transmitting management frames from userspace requires a
>>> frequency. For wpasupp to be race-free during ibss merges, we should have a way to
>>> transmit management frames to the current bss without specifying a frequency...
>>
>> That seems reasonable.
>>
> 
> This is a common problem I faced in other contexts too...

I sent a patch shorty after this message, trying to solve the problem, except Johannes
pointed out that it could still introduce races, at least in some non-mac80211 drivers,
the most problematic being ath6kl. I don't have any ath6kl hardware here, and i don't like
patching things that i can't test.

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 2372 bytes --]

On Mon, Apr 08, 2013 at 12:53:55AM -0700, Nicolas Cavallari wrote:
> On 07/04/2013 23:17, Antonio Quartulli wrote:
> > Hi all,
> > 
> > let's resurrect this thread :)
> > 
> > On Thu, Jan 31, 2013 at 06:26:13AM -0800, Johannes Berg wrote:
> >> On Thu, 2013-01-31 at 15:18 +0100, Nicolas Cavallari wrote:
> >>
> >>>> I think that's acceptable, but if it requires a wpa_s change anyway we
> >>>> could just implement reboot detection there instead of adding all these
> >>>> new events etc.? I.e. rather than having a new supplicant say "OK I will
> >>>> listen to the right event when handling reboot detection", it could just
> >>>> use the existing infrastructure and implement it itself?
> >>>
> >>> Well i already have wpa_supplicant patches for that. Might just need to clean that up a
> >>> bit so it's at least configurable.
> >>>
> > 
> > Would you mind sharing this code?
> 
> Well yes, i just need to untangle it with some other local policies ;)
> 
> > Maybe I can help and offload some work. :)
> > Anyhow I did not understand how you can detect a node reboot from userspace if, as
> > you stated, wpa_s is not able to distinguish encrypted from un-encrypted frames.
> 

Ah ok. I thought we were not talking about registering for AUTH frames here. Got
it now.


I'm wondering why it is better to implement kernel reboot detection in
userspace, but I think it is cleaner and less racy...Then I think it is better
to revert the "delete and add station" mechanism we currently have.

> 
> >>> But now i'm reminded that transmitting management frames from userspace requires a
> >>> frequency. For wpasupp to be race-free during ibss merges, we should have a way to
> >>> transmit management frames to the current bss without specifying a frequency...
> >>
> >> That seems reasonable.
> >>
> > 
> > This is a common problem I faced in other contexts too...
> 
> I sent a patch shorty after this message, trying to solve the problem, except Johannes
> pointed out that it could still introduce races, at least in some non-mac80211 drivers,
> the most problematic being ath6kl. I don't have any ath6kl hardware here, and i don't like
> patching things that i can't test.

Understood. Thanks for the explanation.


Cheers,

-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Antonio Quartulli]

[-- Attachment #1: Type: text/plain, Size: 923 bytes --]

On Thu, Jan 31, 2013 at 06:18:40 -0800, Nicolas Cavallari wrote:
> On 31/01/2013 14:34, Johannes Berg wrote:
> > That seems reasonable. Actually there's no reason for wpa_s to "reset
> > the [...] sta_info", all it really has to do is set it to unauthorized
> > if it detects it was rebooted, and then start key negotiation again, I
> > think?
> 
> I'm not sure why Antonio resetted the sta_info. Not resetting it when a node reboot is
> detected from userspace works for me anyway.

I did that because wpa_s was ignoring the new EAPOL message sent by the rebooted
station. If I am not mistaken wpa_s would not expect a new "unencrypted" key
exchange. When I tried it wa snot working, that's why I came up with the
sta_info reset. But of course, if we could avoid the in-kernel reset, it would
be definitely better.


-- 
Antonio Quartulli

..each of us alone is worth nothing..
Ernesto "Che" Guevara

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] {cfg,nl}80211: tx_mgmt: use current bss channel if omitted.

[Nicolas Cavallari]

Allow not specifying the channel when transmitting a management frame.
This allows user space code to not track the current channel.  This is
especially useful in IBSS mode, because userspace is not informed when
the channel changes because of a merge and requesting the current
channel before using it can introduce races.

Signed-off-by: Nicolas Cavallari <cavallar@lri.fr>
---
 net/wireless/mlme.c    |   14 ++++++++++++++
 net/wireless/nl80211.c |   13 ++++++++++---
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 461e692..8dc3f46 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -757,6 +757,7 @@ int cfg80211_mlme_mgmt_tx(struct cfg80211_registered_device *rdev,
 {
 	const struct ieee80211_mgmt *mgmt;
 	u16 stype;
+	enum cfg80211_chan_mode chan_mode;
 
 	if (!wdev->wiphy->mgmt_stypes)
 		return -EOPNOTSUPP;
@@ -836,10 +837,23 @@ int cfg80211_mlme_mgmt_tx(struct cfg80211_registered_device *rdev,
 			err = -EOPNOTSUPP;
 			break;
 		}
+		if (!err && chan == NULL) {
+			cfg80211_get_chan_state(wdev, &chan, &chan_mode);
+			if (!chan)
+				err = -ENOTCONN;
+		}
 		wdev_unlock(wdev);
 
 		if (err)
 			return err;
+	} else {
+		if (chan == NULL) {
+			wdev_lock(wdev);
+			cfg80211_get_chan_state(wdev, &chan, &chan_mode);
+			wdev_unlock(wdev);
+			if (!chan)
+				return -ENOTCONN;
+		}
 	}
 
 	if (!ether_addr_equal(mgmt->sa, wdev_address(wdev)))
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b5978ab..f22c35e 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -6564,9 +6564,16 @@ static int nl80211_tx_mgmt(struct sk_buff *skb, struct genl_info *info)
 
 	no_cck = nla_get_flag(info->attrs[NL80211_ATTR_TX_NO_CCK_RATE]);
 
-	err = nl80211_parse_chandef(rdev, info, &chandef);
-	if (err)
-		return err;
+	if (info->attrs[NL80211_ATTR_WIPHY_FREQ] ||
+	    info->attrs[NL80211_ATTR_WIPHY_CHANNEL_TYPE]) {
+		err = nl80211_parse_chandef(rdev, info, &chandef);
+		if (err)
+			return err;
+	} else {
+		if (offchan)
+			return -EINVAL;
+		chandef.chan = NULL;
+	}
 
 	if (!dont_wait_for_ack) {
 		msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
-- 
1.7.10.4

Re: [PATCH] {cfg,nl}80211: tx_mgmt: use current bss channel if omitted.

[Johannes Berg]

On Fri, 2013-02-01 at 18:11 +0100, Nicolas Cavallari wrote:
> Allow not specifying the channel when transmitting a management frame.
> This allows user space code to not track the current channel.  This is
> especially useful in IBSS mode, because userspace is not informed when
> the channel changes because of a merge and requesting the current
> channel before using it can introduce races.


> @@ -836,10 +837,23 @@ int cfg80211_mlme_mgmt_tx(struct cfg80211_registered_device *rdev,
>  			err = -EOPNOTSUPP;
>  			break;
>  		}
> +		if (!err && chan == NULL) {
> +			cfg80211_get_chan_state(wdev, &chan, &chan_mode);

I'm not sure this is the best way of handling it. Is there a reason to
not pass NULL through to the driver(s) (and audit them) instead? This
channel access could be racy for channel changes still, etc.

johannes

Re: [PATCH] {cfg,nl}80211: tx_mgmt: use current bss channel if omitted.

[Nicolas Cavallari]

On 04/02/2013 17:04, Johannes Berg wrote:
> On Fri, 2013-02-01 at 18:11 +0100, Nicolas Cavallari wrote:
>> Allow not specifying the channel when transmitting a management frame.
>> This allows user space code to not track the current channel.  This is
>> especially useful in IBSS mode, because userspace is not informed when
>> the channel changes because of a merge and requesting the current
>> channel before using it can introduce races.
> 
> 
>> @@ -836,10 +837,23 @@ int cfg80211_mlme_mgmt_tx(struct cfg80211_registered_device *rdev,
>>  			err = -EOPNOTSUPP;
>>  			break;
>>  		}
>> +		if (!err && chan == NULL) {
>> +			cfg80211_get_chan_state(wdev, &chan, &chan_mode);
> 
> I'm not sure this is the best way of handling it. Is there a reason to
> not pass NULL through to the driver(s) (and audit them) instead? This
> channel access could be racy for channel changes still, etc.

I couldn't understand how tracing would work with NULL arguments, and while most of the
drivers totally ignore the channel argument or does not read it when offchan is false, the
ath6kl driver actually uses the given channel and ignores the offchannel argument.

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Will Hawkins]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Sorry for the delay responding!

On 01/02/2013 01:32 AM, Antonio Quartulli wrote:
> Hi Johannes,
> 
> On Fri, Dec 28, 2012 at 03:51:08 +0100, Johannes Berg wrote:
>> Anyway this part is really confusing. If userspace is handling
>> auth frames, should the kernel really mark the station as
>> authenticated? What then is the point in handling auth frames in
>> userspace??
>> 
>> Will?
> 
> Actually I don't know. Maybe Will can help us in understanding how
> to handle this point. I simply kept the same behaviour: before this
> (my) patch a station was marked as AUTHenticated in by the kernel,
> even if userspace registered for AUTH frames, and the same I'm
> doing here.

It does *not* appear that Antonio's patch changes the logic that I put
in place with my earlier patch. For me, the userspace application
*authorizes* the station rather than *authenticates*. In other words,
no matter whether or not the userspace application is registered for
AUTH frames, the new station is assumed to be authenticated. Once the
two stations are authenticated the key exchange begins (over AUTH
frames) to determine if the stations are authorized.

Johannes, what are your thoughts on the differences between the
semantics of authenticated and authorized. I am definitely in over my
head so please correct me where I've gone wrong. :-)

> 
> 
>> 
>> Any chance we could converge on a single implementation here?
>> 
> 
> Maybe yes :)
> 
> I think that leaving the station not AUTHenticated and let
> userspace do so would be the best approach..but then we need a way
> to enable userspace to do it :)

I would be okay with this approach. I think that it would be fairly
easy to essentially move the logic up a level and protect the
transition to AUTH rather than AUTHORIZED.
> 
> 
>>> +static void ieee80211_ibss_auth_sta(struct sta_info *sta) +{ +
>>> if (sta->sta_state > IEEE80211_STA_NONE) +		return; + +
>>> sta_info_move_state(sta, IEEE80211_STA_AUTH); +
>>> sta_info_move_state(sta, IEEE80211_STA_ASSOC); +	/* authorize
>>> the station only if the network is not RSN protected. If +	 *
>>> not wait for the userspace to authorize it +	 */
>> 
>> technically, control_port != RSN protected, but I guess for IBSS
>> ... who's going to do WAPI? ;-)
> 
> This is a behaviour I introduced some time ago:
> 
> 1153         sdata->u.ibss.control_port = params->control_port;
> 
> which is set based on NL80211_ATTR_CONTROL_PORT and at the moment,
> in IBSS, I think it is only set by wpa_supplicant when using
> IBSS/RSN..you told me to use control_port instead of creating
> another member..but maybe it is general and could be used for
> something else?

The userspace application that I am writing does use the control port
in IBSS. My goal is to integrate my userspace application with
wpa_supplicant so eventually there will be convergence. In fact,
Antonio's new message is critical to this integration. :-)

> 
>> 
>>> +	cfg80211_ibss_sta(sta->sdata->dev, sta->sta.addr,
>>> GFP_KERNEL);
>> 
>> I'm not really convinced that this event is the right thing to
>> do.
>> 
>> If userspace is handing the auth frame, it already has the event
>> right there, no? It can even reply, even if for some reason it
>> can't send a negative reply because the kernel has already marked
>> the station as authenticated (see above).
>> 
> 
> Here I am missing something. To the best of knowledge,
> wpa_supplicant (which is the only userspace tool providing IBSS/RSN
> and it is not handling AUTH frames. Therefore it will not have any
> event for authentication..
> 

See above.

Thanks for everyone's great work! Happy New Year!

Will

>> If there's no userspace, then RSN can't work anyway, so maybe
>> this isn't really needed?
> 
> what we could do is to "move" the NEW_STA event and use it in this
> point. But I preferred to create a new event type in order to
> distinguish between the case of detecting a new station from the
> case of having a new station ready for key exchange.
> 
>> 
>> Also, a more generic event would make more sense I think?
>> 
> 
> You mean a generic event to be triggered on state change? Yes sure,
> but at the moment it was much more important to reshape the
> interaction between US and KS when using IBSS/RSN.
> 
>>> +	ieee80211_ibss_auth_expire(sdata);
>> 
>> It seems you should have something to trigger the timer too?
>> 
> 
> uhm? I don't understand. This function is invoked in
> ieee80211_sta_merge_ibss() which should be periodically called (if
> I got it correctly...the timer stuff in ibss is not really clear to
> me)
> 
> Thank you very much for your feedbacks! Happy new year :)
> 
> Cheers,
> 
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQEcBAEBAgAGBQJQ5fKkAAoJEIN2rhPI25sW4YsH/1xcWuX3u/6WV2tR1Ke/JfOn
bV1swDRU5F5MioBWTAQw2J6l7efogIIRAvaFC50HzU2P4hbEaCZclF0SQX9AcnjT
vDto/MXrRMfkUtzWAMZbjsa5sZzHQNXbh+fiLr/c1Exg99/2NPNRHSf5MNKhb1ck
iMsna5aJLNyR2Ey0Z7RcfL/U4eQv8JbVubpuTgmZZIH6BRV1t7+nS1b4lxs0yw7e
+JwLOpb9NnyA9+Wk2HSpfHNv5+6Nfeq6Ftj+ANqMLRUuryS+5+ujneE3pg609OEc
3LVdl0jUZ1OE2HjF5LcCzNwJLb3tPr/dfL0z5XyG6L2YFtWyYnycO+ixKOHczKw=
=b3Ot
-----END PGP SIGNATURE-----

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Johannes Berg]

On Thu, 2013-01-03 at 16:05 -0500, Will Hawkins wrote:
> Sorry for the delay responding!

Haha, 1.5 days. I win at delaying ;-(

> >> Anyway this part is really confusing. If userspace is handling
> >> auth frames, should the kernel really mark the station as
> >> authenticated? What then is the point in handling auth frames in
> >> userspace??
> >> 
> >> Will?
> > 
> > Actually I don't know. Maybe Will can help us in understanding how
> > to handle this point. I simply kept the same behaviour: before this
> > (my) patch a station was marked as AUTHenticated in by the kernel,
> > even if userspace registered for AUTH frames, and the same I'm
> > doing here.
> 
> It does *not* appear that Antonio's patch changes the logic that I put
> in place with my earlier patch. For me, the userspace application
> *authorizes* the station rather than *authenticates*. In other words,
> no matter whether or not the userspace application is registered for
> AUTH frames, the new station is assumed to be authenticated. Once the
> two stations are authenticated the key exchange begins (over AUTH
> frames) to determine if the stations are authorized.
> 
> Johannes, what are your thoughts on the differences between the
> semantics of authenticated and authorized. I am definitely in over my
> head so please correct me where I've gone wrong. :-)

That makes sense, you handle the SAE authentication and then mark the
station as authorized, which is perfectly fine. But that really mostly
means you don't care about the authenticated state at all, and there's
no real associated state anyway ...

> I would be okay with this approach. I think that it would be fairly
> easy to essentially move the logic up a level and protect the
> transition to AUTH rather than AUTHORIZED.

Well, you'd have to do both, and we also can't break the userspace API
now.

> > This is a behaviour I introduced some time ago:
> > 
> > 1153         sdata->u.ibss.control_port = params->control_port;
> > 
> > which is set based on NL80211_ATTR_CONTROL_PORT and at the moment,
> > in IBSS, I think it is only set by wpa_supplicant when using
> > IBSS/RSN..you told me to use control_port instead of creating
> > another member..but maybe it is general and could be used for
> > something else?

It could be used for something else.

> The userspace application that I am writing does use the control port
> in IBSS. My goal is to integrate my userspace application with
> wpa_supplicant so eventually there will be convergence. In fact,
> Antonio's new message is critical to this integration. :-)

That I don't understand? Why does it need the new event? Wouldn't
completing the SAE handshake be sufficient information?

johannes

Re: [PATCHv3 2/2] mac80211: in AD-HOC mode wait for the AUTH response

[Will Hawkins]

On 01/25/2013 04:45 PM, Johannes Berg wrote:
> On Thu, 2013-01-03 at 16:05 -0500, Will Hawkins wrote:
>> Sorry for the delay responding!
> 
> Haha, 1.5 days. I win at delaying ;-(
> 
>>>> Anyway this part is really confusing. If userspace is handling
>>>> auth frames, should the kernel really mark the station as
>>>> authenticated? What then is the point in handling auth frames in
>>>> userspace??
>>>>
>>>> Will?
>>>
>>> Actually I don't know. Maybe Will can help us in understanding how
>>> to handle this point. I simply kept the same behaviour: before this
>>> (my) patch a station was marked as AUTHenticated in by the kernel,
>>> even if userspace registered for AUTH frames, and the same I'm
>>> doing here.
>>
>> It does *not* appear that Antonio's patch changes the logic that I put
>> in place with my earlier patch. For me, the userspace application
>> *authorizes* the station rather than *authenticates*. In other words,
>> no matter whether or not the userspace application is registered for
>> AUTH frames, the new station is assumed to be authenticated. Once the
>> two stations are authenticated the key exchange begins (over AUTH
>> frames) to determine if the stations are authorized.
>>
>> Johannes, what are your thoughts on the differences between the
>> semantics of authenticated and authorized. I am definitely in over my
>> head so please correct me where I've gone wrong. :-)
> 
> That makes sense, you handle the SAE authentication and then mark the
> station as authorized, which is perfectly fine. But that really mostly
> means you don't care about the authenticated state at all, and there's
> no real associated state anyway ...
> 
>> I would be okay with this approach. I think that it would be fairly
>> easy to essentially move the logic up a level and protect the
>> transition to AUTH rather than AUTHORIZED.
> 
> Well, you'd have to do both, and we also can't break the userspace API
> now.
> 
>>> This is a behaviour I introduced some time ago:
>>>
>>> 1153         sdata->u.ibss.control_port = params->control_port;
>>>
>>> which is set based on NL80211_ATTR_CONTROL_PORT and at the moment,
>>> in IBSS, I think it is only set by wpa_supplicant when using
>>> IBSS/RSN..you told me to use control_port instead of creating
>>> another member..but maybe it is general and could be used for
>>> something else?
> 
> It could be used for something else.
> 
>> The userspace application that I am writing does use the control port
>> in IBSS. My goal is to integrate my userspace application with
>> wpa_supplicant so eventually there will be convergence. In fact,
>> Antonio's new message is critical to this integration. :-)
> 
> That I don't understand? Why does it need the new event? Wouldn't
> completing the SAE handshake be sufficient information?

It does not need the new event, necessarily. I was just hoping to use
that event to know when to kick off the SAE handshake with a new node.
Something along the lines of NL80211_CMD_NEW_PEER_CANDIDATE in the world
of 802.11s.

With the code as it stands, I plan on using the NEW_STA event to start
that process, but I was hoping for something that would indicate the
station was authenticated (in case there was ever a time that we wanted
to layer SAE on top of explicit authentication).

Will

> 
> johannes
> 
>