[PATCH] libertas: Reduce the WPA key installation time.

[PATCH] libertas: Reduce the WPA key installation time.

[Javier Cardona]

WPA requires that the PTK is installed immediately after the 4-way handshake
in order to properly decrypt the subsequent incoming EAPOL-GTK frame.  If the
PTK is not enabled by the time the EAPOL-GTK frame arrives, the frame is
dropped and the supplicant does not receive the group key.

This will happen with fast Access Points that send the EAPOL-GTK frame before
the suplicant has successfully installed and enabled the PTK.  To mitigate
this situation, this patch simplifies and accelerates the SIOCSIWENCODEEXT
execution.

This patch resolves OLPC ticket 7690 (http://dev.laptop.org/ticket/7690)

Signed-off-by: Javier Cardona <javier@cozybit.com>
---
 drivers/net/wireless/libertas/wext.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/libertas/wext.c b/drivers/net/wireless/libertas/wext.c
index d86fcf0..1156be5 100644
--- a/drivers/net/wireless/libertas/wext.c
+++ b/drivers/net/wireless/libertas/wext.c
@@ -30,6 +30,14 @@ static inline void lbs_postpone_association_work(struct lbs_private *priv)
 	queue_delayed_work(priv->work_thread, &priv->assoc_work, HZ / 2);
 }
 
+static inline void lbs_do_association_work(struct lbs_private *priv)
+{
+	if (priv->surpriseremoved)
+		return;
+	cancel_delayed_work(&priv->assoc_work);
+	queue_delayed_work(priv->work_thread, &priv->assoc_work, 0);
+}
+
 static inline void lbs_cancel_association_work(struct lbs_private *priv)
 {
 	cancel_delayed_work(&priv->assoc_work);
@@ -1585,12 +1593,14 @@ static int lbs_set_encodeext(struct net_device *dev,
 			set_bit(ASSOC_FLAG_SECINFO, &assoc_req->flags);
 		}
 
-		disable_wep (assoc_req);
+		/* Only disable wep if necessary: can't waste time here. */
+		if (priv->mac_control & CMD_ACT_MAC_WEP_ENABLE)
+			disable_wep(assoc_req);
 	}
 
 out:
-	if (ret == 0) {
-		lbs_postpone_association_work(priv);
+	if (ret == 0) {  /* key installation is time critical: postpone not! */
+		lbs_do_association_work(priv);
 	} else {
 		lbs_cancel_association_work(priv);
 	}
-- 
1.5.2.4

Re: [PATCH] libertas: Reduce the WPA key installation time.

[Javier Cardona]

John,

On Thu, Sep 11, 2008 at 3:32 PM, Javier Cardona <javier@cozybit.com> wrote:
> This patch resolves OLPC ticket 7690 (http://dev.laptop.org/ticket/7690)

...should say

This patch resolves OLPC ticket 7825 (http://dev.laptop.org/ticket/7825)

Let me know if you want me to resend the patch with that correction.

Cheers,

Javier

Re: [PATCH] libertas: Reduce the WPA key installation time.

[Dan Williams]

On Thu, 2008-09-11 at 15:32 -0700, Javier Cardona wrote:
> WPA requires that the PTK is installed immediately after the 4-way handshake
> in order to properly decrypt the subsequent incoming EAPOL-GTK frame.  If the
> PTK is not enabled by the time the EAPOL-GTK frame arrives, the frame is
> dropped and the supplicant does not receive the group key.
> 
> This will happen with fast Access Points that send the EAPOL-GTK frame before
> the suplicant has successfully installed and enabled the PTK.  To mitigate
> this situation, this patch simplifies and accelerates the SIOCSIWENCODEEXT
> execution.

Good catch though this would break ordering of iwconfig commands like:

iwconfig eth0 essid foobar key 235232363632 mode managed channel 11

which would punt the mode + channel arguments to another association
request, thus causing the first one to either fail (because the mode
wouldn't be switched yet) or to get the wrong AP on the wrong channel
(because the channel lock request wouldn't come until after it had tried
to associate on channels 1 - 10, and if it found a matching AP not on
channel 11 it would use that first).

The problem is of course that the patch unconditionally sets the key
immediately, but that was also the solution to the OLPC bug.
Unfortunately this is something we can't really handle well with WEXT,
but we can try to work around it by testing the following:

1) in infrastructure mode AND one or both of (2) or (3)
2) WPA is on 
3) key management (via SIWAUTH) is one of
	a) IW_AUTH_KEY_MGMT_802_1X
	b) IW_AUTH_KEY_MGMT_PSK

and if this is true, set keys immediately, otherwise punt to the
association handler.

Does something like the following (based off your patch) still fix the
bug?

Dan

diff --git a/drivers/net/wireless/libertas/dev.h b/drivers/net/wireless/libertas/dev.h
index fd59e18..6d2bf5c 100644
--- a/drivers/net/wireless/libertas/dev.h
+++ b/drivers/net/wireless/libertas/dev.h
@@ -58,6 +58,7 @@ struct lbs_802_11_security {
 	u8 WPA2enabled;
 	u8 wep_enabled;
 	u8 auth_mode;
+	u32 key_mgmt;
 };
 
 /** Current Basic Service Set State Structure */
diff --git a/drivers/net/wireless/libertas/wext.c b/drivers/net/wireless/libertas/wext.c
index 426f1fe..43ba998 100644
--- a/drivers/net/wireless/libertas/wext.c
+++ b/drivers/net/wireless/libertas/wext.c
@@ -30,6 +30,14 @@ static inline void lbs_postpone_association_work(struct lbs_private *priv)
 	queue_delayed_work(priv->work_thread, &priv->assoc_work, HZ / 2);
 }
 
+static inline void lbs_do_association_work(struct lbs_private *priv)
+{
+	if (priv->surpriseremoved)
+		return;
+	cancel_delayed_work(&priv->assoc_work);
+	queue_delayed_work(priv->work_thread, &priv->assoc_work, 0);
+}
+
 static inline void lbs_cancel_association_work(struct lbs_private *priv)
 {
 	cancel_delayed_work(&priv->assoc_work);
@@ -1578,12 +1586,27 @@ static int lbs_set_encodeext(struct net_device *dev,
 			set_bit(ASSOC_FLAG_SECINFO, &assoc_req->flags);
 		}
 
+		/* Only disable wep if necessary: can't waste time here. */
+		if (priv->mac_control & CMD_ACT_MAC_WEP_ENABLE)
+			disable_wep(assoc_req);
 		disable_wep (assoc_req);
 	}
 
 out:
 	if (ret == 0) {
-		lbs_postpone_association_work(priv);
+		/* 802.1x and WPA rekeying must happen as quickly as possible,
+		 * especially during the 4-way handshake; thus if in
+		 * infrastructure mode, and either (a) 802.1x is enabled or
+		 * (b) WPA is being used, set the key right away.
+		 */
+		if (assoc_req->mode == IW_MODE_INFRA &&
+		    ((assoc_req->secinfo.key_mgmt & IW_AUTH_KEY_MGMT_802_1X) ||
+		     (assoc_req->secinfo.key_mgmt & IW_AUTH_KEY_MGMT_PSK) ||
+		      assoc_req->secinfo.WPAenabled ||
+		      assoc_req->secinfo.WPA2enabled)) {
+			lbs_do_association_work(priv);
+		} else
+			lbs_postpone_association_work(priv);
 	} else {
 		lbs_cancel_association_work(priv);
 	}
@@ -1691,13 +1714,17 @@ static int lbs_set_auth(struct net_device *dev,
 	case IW_AUTH_TKIP_COUNTERMEASURES:
 	case IW_AUTH_CIPHER_PAIRWISE:
 	case IW_AUTH_CIPHER_GROUP:
-	case IW_AUTH_KEY_MGMT:
 	case IW_AUTH_DROP_UNENCRYPTED:
 		/*
 		 * libertas does not use these parameters
 		 */
 		break;
 
+	case IW_AUTH_KEY_MGMT:
+		assoc_req->secinfo.key_mgmt = dwrq->value;
+		updated = 1;
+		break;
+
 	case IW_AUTH_WPA_VERSION:
 		if (dwrq->value & IW_AUTH_WPA_VERSION_DISABLED) {
 			assoc_req->secinfo.WPAenabled = 0;
@@ -1777,6 +1804,10 @@ static int lbs_get_auth(struct net_device *dev,
 	lbs_deb_enter(LBS_DEB_WEXT);
 
 	switch (dwrq->flags & IW_AUTH_INDEX) {
+	case IW_AUTH_KEY_MGMT:
+		dwrq->value = priv->secinfo.key_mgmt;
+		break;
+
 	case IW_AUTH_WPA_VERSION:
 		dwrq->value = 0;
 		if (priv->secinfo.WPAenabled)

Re: [PATCH] libertas: Reduce the WPA key installation time.

[Dan Williams]

On Mon, 2008-09-15 at 02:46 -0400, Dan Williams wrote:
> On Thu, 2008-09-11 at 15:32 -0700, Javier Cardona wrote:
> > WPA requires that the PTK is installed immediately after the 4-way handshake
> > in order to properly decrypt the subsequent incoming EAPOL-GTK frame.  If the
> > PTK is not enabled by the time the EAPOL-GTK frame arrives, the frame is
> > dropped and the supplicant does not receive the group key.
> > 
> > This will happen with fast Access Points that send the EAPOL-GTK frame before
> > the suplicant has successfully installed and enabled the PTK.  To mitigate
> > this situation, this patch simplifies and accelerates the SIOCSIWENCODEEXT
> > execution.
> 
> Good catch though this would break ordering of iwconfig commands like:
> 
> iwconfig eth0 essid foobar key 235232363632 mode managed channel 11
> 
> which would punt the mode + channel arguments to another association
> request, thus causing the first one to either fail (because the mode
> wouldn't be switched yet) or to get the wrong AP on the wrong channel
> (because the channel lock request wouldn't come until after it had tried
> to associate on channels 1 - 10, and if it found a matching AP not on
> channel 11 it would use that first).
> 
> The problem is of course that the patch unconditionally sets the key
> immediately, but that was also the solution to the OLPC bug.
> Unfortunately this is something we can't really handle well with WEXT,
> but we can try to work around it by testing the following:
> 
> 1) in infrastructure mode AND one or both of (2) or (3)
> 2) WPA is on 
> 3) key management (via SIWAUTH) is one of
> 	a) IW_AUTH_KEY_MGMT_802_1X
> 	b) IW_AUTH_KEY_MGMT_PSK
> 
> and if this is true, set keys immediately, otherwise punt to the
> association handler.
> 
> Does something like the following (based off your patch) still fix the
> bug?
> 
> Dan
> 
> diff --git a/drivers/net/wireless/libertas/dev.h b/drivers/net/wireless/libertas/dev.h
> index fd59e18..6d2bf5c 100644
> --- a/drivers/net/wireless/libertas/dev.h
> +++ b/drivers/net/wireless/libertas/dev.h
> @@ -58,6 +58,7 @@ struct lbs_802_11_security {
>  	u8 WPA2enabled;
>  	u8 wep_enabled;
>  	u8 auth_mode;
> +	u32 key_mgmt;
>  };
>  
>  /** Current Basic Service Set State Structure */
> diff --git a/drivers/net/wireless/libertas/wext.c b/drivers/net/wireless/libertas/wext.c
> index 426f1fe..43ba998 100644
> --- a/drivers/net/wireless/libertas/wext.c
> +++ b/drivers/net/wireless/libertas/wext.c
> @@ -30,6 +30,14 @@ static inline void lbs_postpone_association_work(struct lbs_private *priv)
>  	queue_delayed_work(priv->work_thread, &priv->assoc_work, HZ / 2);
>  }
>  
> +static inline void lbs_do_association_work(struct lbs_private *priv)
> +{
> +	if (priv->surpriseremoved)
> +		return;
> +	cancel_delayed_work(&priv->assoc_work);
> +	queue_delayed_work(priv->work_thread, &priv->assoc_work, 0);
> +}
> +
>  static inline void lbs_cancel_association_work(struct lbs_private *priv)
>  {
>  	cancel_delayed_work(&priv->assoc_work);
> @@ -1578,12 +1586,27 @@ static int lbs_set_encodeext(struct net_device *dev,
>  			set_bit(ASSOC_FLAG_SECINFO, &assoc_req->flags);
>  		}
>  
> +		/* Only disable wep if necessary: can't waste time here. */
> +		if (priv->mac_control & CMD_ACT_MAC_WEP_ENABLE)
> +			disable_wep(assoc_req);
>  		disable_wep (assoc_req);

It's late; this second disable_wep() slipped through.  Remove it of
course :)

>  	}
>  
>  out:
>  	if (ret == 0) {
> -		lbs_postpone_association_work(priv);
> +		/* 802.1x and WPA rekeying must happen as quickly as possible,
> +		 * especially during the 4-way handshake; thus if in
> +		 * infrastructure mode, and either (a) 802.1x is enabled or
> +		 * (b) WPA is being used, set the key right away.
> +		 */
> +		if (assoc_req->mode == IW_MODE_INFRA &&
> +		    ((assoc_req->secinfo.key_mgmt & IW_AUTH_KEY_MGMT_802_1X) ||
> +		     (assoc_req->secinfo.key_mgmt & IW_AUTH_KEY_MGMT_PSK) ||
> +		      assoc_req->secinfo.WPAenabled ||
> +		      assoc_req->secinfo.WPA2enabled)) {
> +			lbs_do_association_work(priv);
> +		} else
> +			lbs_postpone_association_work(priv);
>  	} else {
>  		lbs_cancel_association_work(priv);
>  	}
> @@ -1691,13 +1714,17 @@ static int lbs_set_auth(struct net_device *dev,
>  	case IW_AUTH_TKIP_COUNTERMEASURES:
>  	case IW_AUTH_CIPHER_PAIRWISE:
>  	case IW_AUTH_CIPHER_GROUP:
> -	case IW_AUTH_KEY_MGMT:
>  	case IW_AUTH_DROP_UNENCRYPTED:
>  		/*
>  		 * libertas does not use these parameters
>  		 */
>  		break;
>  
> +	case IW_AUTH_KEY_MGMT:
> +		assoc_req->secinfo.key_mgmt = dwrq->value;
> +		updated = 1;
> +		break;
> +
>  	case IW_AUTH_WPA_VERSION:
>  		if (dwrq->value & IW_AUTH_WPA_VERSION_DISABLED) {
>  			assoc_req->secinfo.WPAenabled = 0;
> @@ -1777,6 +1804,10 @@ static int lbs_get_auth(struct net_device *dev,
>  	lbs_deb_enter(LBS_DEB_WEXT);
>  
>  	switch (dwrq->flags & IW_AUTH_INDEX) {
> +	case IW_AUTH_KEY_MGMT:
> +		dwrq->value = priv->secinfo.key_mgmt;
> +		break;
> +
>  	case IW_AUTH_WPA_VERSION:
>  		dwrq->value = 0;
>  		if (priv->secinfo.WPAenabled)
> 
> 
> --
> 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