linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c
@ 2012-09-06 17:07 Antonio Quartulli
  2012-09-06 17:07 ` [PATCHv2 2/2] mac80211: reply to AUTH frame with a DEAUTH if sta allocation fails Antonio Quartulli
  2012-09-07  8:25 ` [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c Johannes Berg
  0 siblings, 2 replies; 9+ messages in thread
From: Antonio Quartulli @ 2012-09-06 17:07 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Antonio Quartulli

ieee80211_send_deauth_disassoc() is now defined in util.c and it is available
for usage in the rest of the mac80211 code.

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---

v2:
- in ieee80211_send_deauth_disassoc(), limit check for IEEE80211_STA_MFP_ENABLED
  to the case of vif.type equal to STA


 net/mac80211/ieee80211_i.h |  5 ++++
 net/mac80211/mlme.c        | 60 ++++++++--------------------------------------
 net/mac80211/util.c        | 40 +++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 50 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index b95fa25..8874523 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -68,6 +68,8 @@ struct ieee80211_local;
 #define IEEE80211_DEFAULT_MAX_SP_LEN		\
 	IEEE80211_WMM_IE_STA_QOSINFO_SP_ALL
 
+#define IEEE80211_DEAUTH_FRAME_LEN	(24 /* hdr */ + 2 /* reason */)
+
 struct ieee80211_fragment_entry {
 	unsigned long first_frag_time;
 	unsigned int seq;
@@ -1458,6 +1460,9 @@ void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
 			 u16 transaction, u16 auth_alg,
 			 u8 *extra, size_t extra_len, const u8 *bssid,
 			 const u8 *da, const u8 *key, u8 key_len, u8 key_idx);
+void ieee80211_send_deauth_disassoc(struct ieee80211_sub_if_data *sdata,
+				    const u8 *bssid, u16 stype, u16 reason,
+				    bool send_frame, u8 *frame_buf);
 int ieee80211_build_preq_ies(struct ieee80211_local *local, u8 *buffer,
 			     const u8 *ie, size_t ie_len,
 			     enum ieee80211_band band, u32 rate_mask,
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 8746694..0ca3413 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -88,8 +88,6 @@ MODULE_PARM_DESC(probe_wait_ms,
 #define TMR_RUNNING_TIMER	0
 #define TMR_RUNNING_CHANSW	1
 
-#define DEAUTH_DISASSOC_LEN	(24 /* hdr */ + 2 /* reason */)
-
 /*
  * All cfg80211 functions have to be called outside a locked
  * section so that they can acquire a lock themselves... This
@@ -574,46 +572,6 @@ static void ieee80211_send_assoc(struct ieee80211_sub_if_data *sdata)
 	ieee80211_tx_skb(sdata, skb);
 }
 
-static void ieee80211_send_deauth_disassoc(struct ieee80211_sub_if_data *sdata,
-					   const u8 *bssid, u16 stype,
-					   u16 reason, bool send_frame,
-					   u8 *frame_buf)
-{
-	struct ieee80211_local *local = sdata->local;
-	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
-	struct sk_buff *skb;
-	struct ieee80211_mgmt *mgmt = (void *)frame_buf;
-
-	/* build frame */
-	mgmt->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT | stype);
-	mgmt->duration = 0; /* initialize only */
-	mgmt->seq_ctrl = 0; /* initialize only */
-	memcpy(mgmt->da, bssid, ETH_ALEN);
-	memcpy(mgmt->sa, sdata->vif.addr, ETH_ALEN);
-	memcpy(mgmt->bssid, bssid, ETH_ALEN);
-	/* u.deauth.reason_code == u.disassoc.reason_code */
-	mgmt->u.deauth.reason_code = cpu_to_le16(reason);
-
-	if (send_frame) {
-		skb = dev_alloc_skb(local->hw.extra_tx_headroom +
-				    DEAUTH_DISASSOC_LEN);
-		if (!skb)
-			return;
-
-		skb_reserve(skb, local->hw.extra_tx_headroom);
-
-		/* copy in frame */
-		memcpy(skb_put(skb, DEAUTH_DISASSOC_LEN),
-		       mgmt, DEAUTH_DISASSOC_LEN);
-
-		if (!(ifmgd->flags & IEEE80211_STA_MFP_ENABLED))
-			IEEE80211_SKB_CB(skb)->flags |=
-				IEEE80211_TX_INTFL_DONT_ENCRYPT;
-
-		ieee80211_tx_skb(sdata, skb);
-	}
-}
-
 void ieee80211_send_pspoll(struct ieee80211_local *local,
 			   struct ieee80211_sub_if_data *sdata)
 {
@@ -1695,7 +1653,7 @@ static void __ieee80211_disconnect(struct ieee80211_sub_if_data *sdata,
 {
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 	struct ieee80211_local *local = sdata->local;
-	u8 frame_buf[DEAUTH_DISASSOC_LEN];
+	u8 frame_buf[IEEE80211_DEAUTH_FRAME_LEN];
 
 	mutex_lock(&ifmgd->mtx);
 	if (!ifmgd->associated) {
@@ -1713,7 +1671,7 @@ static void __ieee80211_disconnect(struct ieee80211_sub_if_data *sdata,
 	 * must be outside lock due to cfg80211,
 	 * but that's not a problem.
 	 */
-	cfg80211_send_deauth(sdata->dev, frame_buf, DEAUTH_DISASSOC_LEN);
+	cfg80211_send_deauth(sdata->dev, frame_buf, IEEE80211_DEAUTH_FRAME_LEN);
 
 	mutex_lock(&local->mtx);
 	ieee80211_recalc_idle(local);
@@ -2645,7 +2603,7 @@ static void ieee80211_sta_connection_lost(struct ieee80211_sub_if_data *sdata,
 {
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
-	u8 frame_buf[DEAUTH_DISASSOC_LEN];
+	u8 frame_buf[IEEE80211_DEAUTH_FRAME_LEN];
 
 	ieee80211_set_disassoc(sdata, IEEE80211_STYPE_DEAUTH, reason,
 			       false, frame_buf);
@@ -2655,7 +2613,7 @@ static void ieee80211_sta_connection_lost(struct ieee80211_sub_if_data *sdata,
 	 * must be outside lock due to cfg80211,
 	 * but that's not a problem.
 	 */
-	cfg80211_send_deauth(sdata->dev, frame_buf, DEAUTH_DISASSOC_LEN);
+	cfg80211_send_deauth(sdata->dev, frame_buf, IEEE80211_DEAUTH_FRAME_LEN);
 
 	mutex_lock(&local->mtx);
 	ieee80211_recalc_idle(local);
@@ -3538,7 +3496,7 @@ int ieee80211_mgd_deauth(struct ieee80211_sub_if_data *sdata,
 			 struct cfg80211_deauth_request *req)
 {
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
-	u8 frame_buf[DEAUTH_DISASSOC_LEN];
+	u8 frame_buf[IEEE80211_DEAUTH_FRAME_LEN];
 
 	mutex_lock(&ifmgd->mtx);
 
@@ -3566,7 +3524,8 @@ int ieee80211_mgd_deauth(struct ieee80211_sub_if_data *sdata,
 
 	mutex_unlock(&ifmgd->mtx);
 
-	__cfg80211_send_deauth(sdata->dev, frame_buf, DEAUTH_DISASSOC_LEN);
+	__cfg80211_send_deauth(sdata->dev, frame_buf,
+			       IEEE80211_DEAUTH_FRAME_LEN);
 
 	mutex_lock(&sdata->local->mtx);
 	ieee80211_recalc_idle(sdata->local);
@@ -3580,7 +3539,7 @@ int ieee80211_mgd_disassoc(struct ieee80211_sub_if_data *sdata,
 {
 	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
 	u8 bssid[ETH_ALEN];
-	u8 frame_buf[DEAUTH_DISASSOC_LEN];
+	u8 frame_buf[IEEE80211_DEAUTH_FRAME_LEN];
 
 	mutex_lock(&ifmgd->mtx);
 
@@ -3605,7 +3564,8 @@ int ieee80211_mgd_disassoc(struct ieee80211_sub_if_data *sdata,
 			       frame_buf);
 	mutex_unlock(&ifmgd->mtx);
 
-	__cfg80211_send_disassoc(sdata->dev, frame_buf, DEAUTH_DISASSOC_LEN);
+	__cfg80211_send_disassoc(sdata->dev, frame_buf,
+				 IEEE80211_DEAUTH_FRAME_LEN);
 
 	mutex_lock(&sdata->local->mtx);
 	ieee80211_recalc_idle(sdata->local);
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index ed75439..2d74fad 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1007,6 +1007,46 @@ void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
 	ieee80211_tx_skb(sdata, skb);
 }
 
+void ieee80211_send_deauth_disassoc(struct ieee80211_sub_if_data *sdata,
+				    const u8 *bssid, u16 stype, u16 reason,
+				    bool send_frame, u8 *frame_buf)
+{
+	struct ieee80211_local *local = sdata->local;
+	struct sk_buff *skb;
+	struct ieee80211_mgmt *mgmt = (void *)frame_buf;
+
+	/* build frame */
+	mgmt->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT | stype);
+	mgmt->duration = 0; /* initialize only */
+	mgmt->seq_ctrl = 0; /* initialize only */
+	memcpy(mgmt->da, bssid, ETH_ALEN);
+	memcpy(mgmt->sa, sdata->vif.addr, ETH_ALEN);
+	memcpy(mgmt->bssid, bssid, ETH_ALEN);
+	/* u.deauth.reason_code == u.disassoc.reason_code */
+	mgmt->u.deauth.reason_code = cpu_to_le16(reason);
+
+	if (send_frame) {
+		skb = dev_alloc_skb(local->hw.extra_tx_headroom +
+				    IEEE80211_DEAUTH_FRAME_LEN);
+		if (!skb)
+			return;
+
+		skb_reserve(skb, local->hw.extra_tx_headroom);
+
+		/* copy in frame */
+		memcpy(skb_put(skb, IEEE80211_DEAUTH_FRAME_LEN),
+		       mgmt, IEEE80211_DEAUTH_FRAME_LEN);
+
+		if (sdata->vif.type == NL80211_IFTYPE_STATION &&
+		    !(sdata->u.mgd.flags & IEEE80211_STA_MFP_ENABLED))
+			IEEE80211_SKB_CB(skb)->flags |=
+				IEEE80211_TX_INTFL_DONT_ENCRYPT;
+
+		ieee80211_tx_skb(sdata, skb);
+	}
+}
+
+
 int ieee80211_build_preq_ies(struct ieee80211_local *local, u8 *buffer,
 			     const u8 *ie, size_t ie_len,
 			     enum ieee80211_band band, u32 rate_mask,
-- 
1.7.12


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCHv2 2/2] mac80211: reply to AUTH frame with a DEAUTH if sta allocation fails
  2012-09-06 17:07 [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c Antonio Quartulli
@ 2012-09-06 17:07 ` Antonio Quartulli
  2012-09-07  8:25 ` [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c Johannes Berg
  1 sibling, 0 replies; 9+ messages in thread
From: Antonio Quartulli @ 2012-09-06 17:07 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Antonio Quartulli

Whenever a host gets an AUTH frame it first allocates a new station and then
replies with another AUTH frame. However, if sta allocations fails the host
should send a DEAUTH frame instead to tell the other end that something went
wrong

Signed-off-by: Antonio Quartulli <ordex@autistici.org>
---

v2:
- noop

 net/mac80211/ibss.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index a9d9328..7c08251 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -332,11 +332,27 @@ ieee80211_ibss_add_sta(struct ieee80211_sub_if_data *sdata,
 	return ieee80211_ibss_finish_sta(sta, auth);
 }
 
+static void ieee80211_rx_mgmt_deauth_ibss(struct ieee80211_sub_if_data *sdata,
+					  struct ieee80211_mgmt *mgmt,
+					  size_t len)
+{
+	u16 reason = le16_to_cpu(mgmt->u.deauth.reason_code);
+
+	if (len < IEEE80211_DEAUTH_FRAME_LEN)
+		return;
+
+	ibss_dbg(sdata, "RX DeAuth SA=%pM DA=%pM BSSID=%pM (reason: %d)\n",
+		 mgmt->sa, mgmt->da, mgmt->bssid, reason);
+	sta_info_destroy_addr(sdata, mgmt->sa);
+}
+
 static void ieee80211_rx_mgmt_auth_ibss(struct ieee80211_sub_if_data *sdata,
 					struct ieee80211_mgmt *mgmt,
 					size_t len)
 {
 	u16 auth_alg, auth_transaction;
+	struct sta_info *sta;
+	u8 deauth_frame_buf[IEEE80211_DEAUTH_FRAME_LEN];
 
 	lockdep_assert_held(&sdata->u.ibss.mtx);
 
@@ -352,10 +368,22 @@ 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);
 	sta_info_destroy_addr(sdata, mgmt->sa);
-	ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, 0, false);
+	sta = ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, 0, false);
 	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
+	 */
+	if (!sta) {
+		ieee80211_send_deauth_disassoc(sdata, sdata->u.ibss.bssid,
+					       IEEE80211_STYPE_DEAUTH,
+					       WLAN_REASON_UNSPECIFIED, true,
+					       deauth_frame_buf);
+		return;
+	}
+
+	/*
 	 * IEEE 802.11 standard does not require authentication in IBSS
 	 * networks and most implementations do not seem to use it.
 	 * However, try to reply to authentication attempts if someone
@@ -902,6 +930,9 @@ void ieee80211_ibss_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
 	case IEEE80211_STYPE_AUTH:
 		ieee80211_rx_mgmt_auth_ibss(sdata, mgmt, skb->len);
 		break;
+	case IEEE80211_STYPE_DEAUTH:
+		ieee80211_rx_mgmt_deauth_ibss(sdata, mgmt, skb->len);
+		break;
 	}
 
  mgmt_out:
-- 
1.7.12


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c
  2012-09-06 17:07 [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c Antonio Quartulli
  2012-09-06 17:07 ` [PATCHv2 2/2] mac80211: reply to AUTH frame with a DEAUTH if sta allocation fails Antonio Quartulli
@ 2012-09-07  8:25 ` Johannes Berg
  2012-09-07  9:50   ` Nicolas Cavallari
  1 sibling, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2012-09-07  8:25 UTC (permalink / raw)
  To: Antonio Quartulli; +Cc: John W. Linville, linux-wireless

On Thu, 2012-09-06 at 19:07 +0200, Antonio Quartulli wrote:

> -		if (!(ifmgd->flags & IEEE80211_STA_MFP_ENABLED))
> -			IEEE80211_SKB_CB(skb)->flags |=
> -				IEEE80211_TX_INTFL_DONT_ENCRYPT;


> +		if (sdata->vif.type == NL80211_IFTYPE_STATION &&
> +		    !(sdata->u.mgd.flags & IEEE80211_STA_MFP_ENABLED))
> +			IEEE80211_SKB_CB(skb)->flags |=
> +				IEEE80211_TX_INTFL_DONT_ENCRYPT;

It would seem that this should be 

	if (sdata->vif.type != NL80211_IFTYPE_STATION ||
	    !(flags & MFP_ENABLED))

?

johannes


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c
  2012-09-07  8:25 ` [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c Johannes Berg
@ 2012-09-07  9:50   ` Nicolas Cavallari
  2012-09-07 11:18     ` Johannes Berg
  0 siblings, 1 reply; 9+ messages in thread
From: Nicolas Cavallari @ 2012-09-07  9:50 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Antonio Quartulli, John W. Linville, linux-wireless

On 07/09/2012 10:25, Johannes Berg wrote:
> On Thu, 2012-09-06 at 19:07 +0200, Antonio Quartulli wrote:
> 
>> -		if (!(ifmgd->flags & IEEE80211_STA_MFP_ENABLED))
>> -			IEEE80211_SKB_CB(skb)->flags |=
>> -				IEEE80211_TX_INTFL_DONT_ENCRYPT;
> 
> 
>> +		if (sdata->vif.type == NL80211_IFTYPE_STATION &&
>> +		    !(sdata->u.mgd.flags & IEEE80211_STA_MFP_ENABLED))
>> +			IEEE80211_SKB_CB(skb)->flags |=
>> +				IEEE80211_TX_INTFL_DONT_ENCRYPT;
> 
> It would seem that this should be 
> 
> 	if (sdata->vif.type != NL80211_IFTYPE_STATION ||
> 	    !(flags & MFP_ENABLED))
> 
> ?

Or just never set TX_INTFL_DONT_ENCRYPT at all and rely on
ieee80211_tx_h_select_key() to do the right thing ?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c
  2012-09-07  9:50   ` Nicolas Cavallari
@ 2012-09-07 11:18     ` Johannes Berg
  2012-09-07 11:21       ` Antonio Quartulli
  2012-09-07 12:01       ` Nicolas Cavallari
  0 siblings, 2 replies; 9+ messages in thread
From: Johannes Berg @ 2012-09-07 11:18 UTC (permalink / raw)
  To: Nicolas Cavallari; +Cc: Antonio Quartulli, John W. Linville, linux-wireless

On Fri, 2012-09-07 at 11:50 +0200, Nicolas Cavallari wrote:
> On 07/09/2012 10:25, Johannes Berg wrote:
> > On Thu, 2012-09-06 at 19:07 +0200, Antonio Quartulli wrote:
> > 
> >> -		if (!(ifmgd->flags & IEEE80211_STA_MFP_ENABLED))
> >> -			IEEE80211_SKB_CB(skb)->flags |=
> >> -				IEEE80211_TX_INTFL_DONT_ENCRYPT;
> > 
> > 
> >> +		if (sdata->vif.type == NL80211_IFTYPE_STATION &&
> >> +		    !(sdata->u.mgd.flags & IEEE80211_STA_MFP_ENABLED))
> >> +			IEEE80211_SKB_CB(skb)->flags |=
> >> +				IEEE80211_TX_INTFL_DONT_ENCRYPT;
> > 
> > It would seem that this should be 
> > 
> > 	if (sdata->vif.type != NL80211_IFTYPE_STATION ||
> > 	    !(flags & MFP_ENABLED))
> > 
> > ?
> 
> Or just never set TX_INTFL_DONT_ENCRYPT at all and rely on
> ieee80211_tx_h_select_key() to do the right thing ?

I don't think it can do the right thing, it doesn't check whether MFP is
enabled or not... unless you want to test all those cases I'd rather not
change it :)

johannes


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c
  2012-09-07 11:18     ` Johannes Berg
@ 2012-09-07 11:21       ` Antonio Quartulli
  2012-09-07 12:01       ` Nicolas Cavallari
  1 sibling, 0 replies; 9+ messages in thread
From: Antonio Quartulli @ 2012-09-07 11:21 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Nicolas Cavallari, John W. Linville, linux-wireless

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

On Fri, Sep 07, 2012 at 01:18:09 +0200, Johannes Berg wrote:
> On Fri, 2012-09-07 at 11:50 +0200, Nicolas Cavallari wrote:
> > On 07/09/2012 10:25, Johannes Berg wrote:
> > > On Thu, 2012-09-06 at 19:07 +0200, Antonio Quartulli wrote:
> > > 
> > >> -		if (!(ifmgd->flags & IEEE80211_STA_MFP_ENABLED))
> > >> -			IEEE80211_SKB_CB(skb)->flags |=
> > >> -				IEEE80211_TX_INTFL_DONT_ENCRYPT;
> > > 
> > > 
> > >> +		if (sdata->vif.type == NL80211_IFTYPE_STATION &&
> > >> +		    !(sdata->u.mgd.flags & IEEE80211_STA_MFP_ENABLED))
> > >> +			IEEE80211_SKB_CB(skb)->flags |=
> > >> +				IEEE80211_TX_INTFL_DONT_ENCRYPT;
> > > 
> > > It would seem that this should be 
> > > 
> > > 	if (sdata->vif.type != NL80211_IFTYPE_STATION ||
> > > 	    !(flags & MFP_ENABLED))
> > > 
> > > ?
> > 
> > Or just never set TX_INTFL_DONT_ENCRYPT at all and rely on
> > ieee80211_tx_h_select_key() to do the right thing ?
> 
> I don't think it can do the right thing, it doesn't check whether MFP is
> enabled or not... unless you want to test all those cases I'd rather not
> change it :)

Ok, then I will send v3 with the modified if-condition.

Thank you all,

-- 
Antonio Quartulli

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

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c
  2012-09-07 11:18     ` Johannes Berg
  2012-09-07 11:21       ` Antonio Quartulli
@ 2012-09-07 12:01       ` Nicolas Cavallari
  2012-09-07 12:05         ` Johannes Berg
  1 sibling, 1 reply; 9+ messages in thread
From: Nicolas Cavallari @ 2012-09-07 12:01 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Antonio Quartulli, John W. Linville, linux-wireless

On 07/09/2012 13:18, Johannes Berg wrote:
> On Fri, 2012-09-07 at 11:50 +0200, Nicolas Cavallari wrote:
>> On 07/09/2012 10:25, Johannes Berg wrote:
>>> On Thu, 2012-09-06 at 19:07 +0200, Antonio Quartulli wrote:
>>>
>>>> -		if (!(ifmgd->flags & IEEE80211_STA_MFP_ENABLED))
>>>> -			IEEE80211_SKB_CB(skb)->flags |=
>>>> -				IEEE80211_TX_INTFL_DONT_ENCRYPT;
>>>
>>>
>>>> +		if (sdata->vif.type == NL80211_IFTYPE_STATION &&
>>>> +		    !(sdata->u.mgd.flags & IEEE80211_STA_MFP_ENABLED))
>>>> +			IEEE80211_SKB_CB(skb)->flags |=
>>>> +				IEEE80211_TX_INTFL_DONT_ENCRYPT;
>>>
>>> It would seem that this should be 
>>>
>>> 	if (sdata->vif.type != NL80211_IFTYPE_STATION ||
>>> 	    !(flags & MFP_ENABLED))
>>>
>>> ?
>>
>> Or just never set TX_INTFL_DONT_ENCRYPT at all and rely on
>> ieee80211_tx_h_select_key() to do the right thing ?
> 
> I don't think it can do the right thing, it doesn't check whether MFP is
> enabled or not...

It does; The first part try to encrypt everything, the second part
disable encryption if ccmp is selected and !ieee80211_is_data_present &&
!ieee80211_use_mfp,
which test, among other things, for the sta's WLAN_STA_MFP flag if sta
!= null.

If tx_h_select_key does not select the right key in this case, i think
we have bigger problems.

> unless you want to test all those cases I'd rather not
> change it :)

Not worth the trouble in this case, but i think there is too much code
that sets TX_INTFL_DONT_ENCRYPT when it shouldn't.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c
  2012-09-07 12:01       ` Nicolas Cavallari
@ 2012-09-07 12:05         ` Johannes Berg
  2012-09-07 12:18           ` Nicolas Cavallari
  0 siblings, 1 reply; 9+ messages in thread
From: Johannes Berg @ 2012-09-07 12:05 UTC (permalink / raw)
  To: Nicolas Cavallari; +Cc: Antonio Quartulli, John W. Linville, linux-wireless

On Fri, 2012-09-07 at 14:01 +0200, Nicolas Cavallari wrote:

> >> Or just never set TX_INTFL_DONT_ENCRYPT at all and rely on
> >> ieee80211_tx_h_select_key() to do the right thing ?
> > 
> > I don't think it can do the right thing, it doesn't check whether MFP is
> > enabled or not...
> 
> It does; The first part try to encrypt everything, the second part
> disable encryption if ccmp is selected and !ieee80211_is_data_present &&
> !ieee80211_use_mfp,
> which test, among other things, for the sta's WLAN_STA_MFP flag if sta
> != null.

Good point.

> If tx_h_select_key does not select the right key in this case, i think
> we have bigger problems.

Maybe, maybe not. But it looks like it would be safe.

> > unless you want to test all those cases I'd rather not
> > change it :)
> 
> Not worth the trouble in this case, but i think there is too much code
> that sets TX_INTFL_DONT_ENCRYPT when it shouldn't.

I already applied the v3 patchset, but even if I hadn't I'd say it
should be a separate patch(set), want to send some patches to remove
them? :)

johannes


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c
  2012-09-07 12:05         ` Johannes Berg
@ 2012-09-07 12:18           ` Nicolas Cavallari
  0 siblings, 0 replies; 9+ messages in thread
From: Nicolas Cavallari @ 2012-09-07 12:18 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Antonio Quartulli, John W. Linville, linux-wireless

On 07/09/2012 14:05, Johannes Berg wrote:
> On Fri, 2012-09-07 at 14:01 +0200, Nicolas Cavallari wrote:
> 
>>>> Or just never set TX_INTFL_DONT_ENCRYPT at all and rely on
>>>> ieee80211_tx_h_select_key() to do the right thing ?
>>>
>>> I don't think it can do the right thing, it doesn't check whether MFP is
>>> enabled or not...
>>
>> It does; The first part try to encrypt everything, the second part
>> disable encryption if ccmp is selected and !ieee80211_is_data_present &&
>> !ieee80211_use_mfp,
>> which test, among other things, for the sta's WLAN_STA_MFP flag if sta
>> != null.
> 
> Good point.
> 
>> If tx_h_select_key does not select the right key in this case, i think
>> we have bigger problems.
> 
> Maybe, maybe not. But it looks like it would be safe.
> 
>>> unless you want to test all those cases I'd rather not
>>> change it :)
>>
>> Not worth the trouble in this case, but i think there is too much code
>> that sets TX_INTFL_DONT_ENCRYPT when it shouldn't.
> 
> I already applied the v3 patchset, but even if I hadn't I'd say it
> should be a separate patch(set), want to send some patches to remove
> them? :)

I'll do that later, along with some other 802.11 2012 crypto work.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-09-07 12:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-06 17:07 [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c Antonio Quartulli
2012-09-06 17:07 ` [PATCHv2 2/2] mac80211: reply to AUTH frame with a DEAUTH if sta allocation fails Antonio Quartulli
2012-09-07  8:25 ` [PATCHv2 1/2] mac80211: export ieee80211_send_deauth_disassoc for usage outside of mlme.c Johannes Berg
2012-09-07  9:50   ` Nicolas Cavallari
2012-09-07 11:18     ` Johannes Berg
2012-09-07 11:21       ` Antonio Quartulli
2012-09-07 12:01       ` Nicolas Cavallari
2012-09-07 12:05         ` Johannes Berg
2012-09-07 12:18           ` Nicolas Cavallari

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).