diff mbox

Question on IBSS + RSN

Message ID 20170114120236.GA6856@w1.fi
State Accepted
Headers show

Commit Message

Jouni Malinen Jan. 14, 2017, 12:02 p.m. UTC
On Fri, Jan 13, 2017 at 02:48:07PM -0800, Ben Greear wrote:
> Ok, seems I should focus on events around Authentication message then.

I was able to reproduce one related issue with mac80211_hwsim. This is a
bug that prevents wpa_supplicant from clearing the key when receiving
the Authentication frame from a peer. The patch below fixes that.
However, this resulted in hitting another issue within mac80211 where it
looks like the unicast frame from the re-joining STA gets lost in the RX
reorder buffer logic.. I'm not completely sure what causes that, but for
initial testing, using disable_ht=1 seemed to be a sufficient workaround
(i.e., disable QoS and reorder buffer logic for BA).


[PATCH] RSN IBSS: Fix TK clearing on Authentication frame RX

When wpa_supplicant was processing a received Authentication frame (seq
1) from a peer STA for which there was already a TK configured to the
driver, debug log claimed that the PTK gets cleared, but the actual
call to clear the key was actually dropped due to AUTH vs. SUPP set_key
selection. Fix this by explicitly clearing the TK in case it was set
and an Authentication frame (seq 1) is received.

This fixes some cases where EAPOL-Key frames were sent encrypted using
the old key when a peer STA restarted itself and lost the key and had to
re-join the IBSS. Previously, that state required timing out the 4-way
handshake and Deauthentication frame exchange to recover.

Signed-off-by: Jouni Malinen <j@w1.fi>
---
 wpa_supplicant/ibss_rsn.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Ben Greear Jan. 14, 2017, 4:41 p.m. UTC | #1
On 01/14/2017 04:02 AM, Jouni Malinen wrote:
> On Fri, Jan 13, 2017 at 02:48:07PM -0800, Ben Greear wrote:
>> Ok, seems I should focus on events around Authentication message then.
>
> I was able to reproduce one related issue with mac80211_hwsim. This is a
> bug that prevents wpa_supplicant from clearing the key when receiving
> the Authentication frame from a peer. The patch below fixes that.
> However, this resulted in hitting another issue within mac80211 where it
> looks like the unicast frame from the re-joining STA gets lost in the RX
> reorder buffer logic.. I'm not completely sure what causes that, but for
> initial testing, using disable_ht=1 seemed to be a sufficient workaround
> (i.e., disable QoS and reorder buffer logic for BA).

Thanks for the quick fix!  I've added this to my builds, and will test it
by Monday.

Just maybe I will not hit the reorder bug since ath10k does BA logic in
the firmware.

In case it is easy, you might try testing the 3.7 kernel to see if it
has the mac80211 bug.  My testing indicates
that things get worse sometime between 4.7 and 4.9, but possibly it
was just this supplicant issue or something else entirely.

Thanks,
Ben

>
>
> [PATCH] RSN IBSS: Fix TK clearing on Authentication frame RX
>
> When wpa_supplicant was processing a received Authentication frame (seq
> 1) from a peer STA for which there was already a TK configured to the
> driver, debug log claimed that the PTK gets cleared, but the actual
> call to clear the key was actually dropped due to AUTH vs. SUPP set_key
> selection. Fix this by explicitly clearing the TK in case it was set
> and an Authentication frame (seq 1) is received.
>
> This fixes some cases where EAPOL-Key frames were sent encrypted using
> the old key when a peer STA restarted itself and lost the key and had to
> re-join the IBSS. Previously, that state required timing out the 4-way
> handshake and Deauthentication frame exchange to recover.
>
> Signed-off-by: Jouni Malinen <j@w1.fi>
> ---
>   wpa_supplicant/ibss_rsn.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/wpa_supplicant/ibss_rsn.c b/wpa_supplicant/ibss_rsn.c
> index 53d7d57..c29d8d3 100644
> --- a/wpa_supplicant/ibss_rsn.c
> +++ b/wpa_supplicant/ibss_rsn.c
> @@ -838,6 +838,18 @@ static void ibss_rsn_handle_auth_1_of_2(struct ibss_rsn *ibss_rsn,
>   		   MAC2STR(addr));
>
>   	if (peer &&
> +	    peer->authentication_status & (IBSS_RSN_SET_PTK_SUPP |
> +					   IBSS_RSN_SET_PTK_AUTH)) {
> +		/* Clear the TK for this pair to allow recovery from the case
> +		 * where the peer STA has restarted and lost its key while we
> +		 * still have a pairwise key configured. */
> +		wpa_printf(MSG_DEBUG, "RSN: Clear pairwise key for peer "
> +			   MACSTR, MAC2STR(addr));
> +		wpa_drv_set_key(ibss_rsn->wpa_s, WPA_ALG_NONE, addr, 0, 0,
> +				NULL, 0, NULL, 0);
> +	}
> +
> +	if (peer &&
>   	    peer->authentication_status & IBSS_RSN_AUTH_EAPOL_BY_PEER) {
>   		if (peer->own_auth_tx.sec) {
>   			struct os_reltime now, diff;
>
Ben Greear Jan. 17, 2017, 9:26 p.m. UTC | #2
On 01/14/2017 04:02 AM, Jouni Malinen wrote:
> On Fri, Jan 13, 2017 at 02:48:07PM -0800, Ben Greear wrote:
>> Ok, seems I should focus on events around Authentication message then.
>
> I was able to reproduce one related issue with mac80211_hwsim. This is a
> bug that prevents wpa_supplicant from clearing the key when receiving
> the Authentication frame from a peer. The patch below fixes that.
> However, this resulted in hitting another issue within mac80211 where it
> looks like the unicast frame from the re-joining STA gets lost in the RX
> reorder buffer logic.. I'm not completely sure what causes that, but for
> initial testing, using disable_ht=1 seemed to be a sufficient workaround
> (i.e., disable QoS and reorder buffer logic for BA).

I added this patch, but my systems still do not work well.

When I tried to put the stations in 'a' mode to disable ht, then the
association response is failing for some reason.

Could you post an example of the config file you had that worked
for you?

Thanks,
Ben


>
>
> [PATCH] RSN IBSS: Fix TK clearing on Authentication frame RX
>
> When wpa_supplicant was processing a received Authentication frame (seq
> 1) from a peer STA for which there was already a TK configured to the
> driver, debug log claimed that the PTK gets cleared, but the actual
> call to clear the key was actually dropped due to AUTH vs. SUPP set_key
> selection. Fix this by explicitly clearing the TK in case it was set
> and an Authentication frame (seq 1) is received.
>
> This fixes some cases where EAPOL-Key frames were sent encrypted using
> the old key when a peer STA restarted itself and lost the key and had to
> re-join the IBSS. Previously, that state required timing out the 4-way
> handshake and Deauthentication frame exchange to recover.
>
> Signed-off-by: Jouni Malinen <j@w1.fi>
> ---
>  wpa_supplicant/ibss_rsn.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/wpa_supplicant/ibss_rsn.c b/wpa_supplicant/ibss_rsn.c
> index 53d7d57..c29d8d3 100644
> --- a/wpa_supplicant/ibss_rsn.c
> +++ b/wpa_supplicant/ibss_rsn.c
> @@ -838,6 +838,18 @@ static void ibss_rsn_handle_auth_1_of_2(struct ibss_rsn *ibss_rsn,
>  		   MAC2STR(addr));
>
>  	if (peer &&
> +	    peer->authentication_status & (IBSS_RSN_SET_PTK_SUPP |
> +					   IBSS_RSN_SET_PTK_AUTH)) {
> +		/* Clear the TK for this pair to allow recovery from the case
> +		 * where the peer STA has restarted and lost its key while we
> +		 * still have a pairwise key configured. */
> +		wpa_printf(MSG_DEBUG, "RSN: Clear pairwise key for peer "
> +			   MACSTR, MAC2STR(addr));
> +		wpa_drv_set_key(ibss_rsn->wpa_s, WPA_ALG_NONE, addr, 0, 0,
> +				NULL, 0, NULL, 0);
> +	}
> +
> +	if (peer &&
>  	    peer->authentication_status & IBSS_RSN_AUTH_EAPOL_BY_PEER) {
>  		if (peer->own_auth_tx.sec) {
>  			struct os_reltime now, diff;
>
Ben Greear Jan. 18, 2017, 1:57 a.m. UTC | #3
I spent some more time debugging this today.

When reloading the driver (and restarting supplicant), sometimes I can get two
ath10k to talk IBSS + RSN, and others it fails.

I instrumented the firmware to dump some state on demand, and it shows the same AST entries
(where it stores pointers to the key data), and TIDs look similar as well
in both the good and failure case.  So, it doesn't seem that the issue is
lack of keys getting to the firmware.  Maybe the keys are wrong, but I am
not sure how I could tell that.

If I dump the actual key data for the peers, is that something I could
compare against supplicant logs and verify the right keys are hitting
the firmware?

Is there anything I could do in supplicant to make the keys less random
just to simplify testing (like make sure the hardware keys should always
be the same value??)

Thanks,
Ben

On 01/17/2017 01:26 PM, Ben Greear wrote:
> On 01/14/2017 04:02 AM, Jouni Malinen wrote:
>> On Fri, Jan 13, 2017 at 02:48:07PM -0800, Ben Greear wrote:
>>> Ok, seems I should focus on events around Authentication message then.
>>
>> I was able to reproduce one related issue with mac80211_hwsim. This is a
>> bug that prevents wpa_supplicant from clearing the key when receiving
>> the Authentication frame from a peer. The patch below fixes that.
>> However, this resulted in hitting another issue within mac80211 where it
>> looks like the unicast frame from the re-joining STA gets lost in the RX
>> reorder buffer logic.. I'm not completely sure what causes that, but for
>> initial testing, using disable_ht=1 seemed to be a sufficient workaround
>> (i.e., disable QoS and reorder buffer logic for BA).
>
> I added this patch, but my systems still do not work well.
>
> When I tried to put the stations in 'a' mode to disable ht, then the
> association response is failing for some reason.
>
> Could you post an example of the config file you had that worked
> for you?
>
> Thanks,
> Ben
>
>
>>
>>
>> [PATCH] RSN IBSS: Fix TK clearing on Authentication frame RX
>>
>> When wpa_supplicant was processing a received Authentication frame (seq
>> 1) from a peer STA for which there was already a TK configured to the
>> driver, debug log claimed that the PTK gets cleared, but the actual
>> call to clear the key was actually dropped due to AUTH vs. SUPP set_key
>> selection. Fix this by explicitly clearing the TK in case it was set
>> and an Authentication frame (seq 1) is received.
>>
>> This fixes some cases where EAPOL-Key frames were sent encrypted using
>> the old key when a peer STA restarted itself and lost the key and had to
>> re-join the IBSS. Previously, that state required timing out the 4-way
>> handshake and Deauthentication frame exchange to recover.
>>
>> Signed-off-by: Jouni Malinen <j@w1.fi>
>> ---
>>  wpa_supplicant/ibss_rsn.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/wpa_supplicant/ibss_rsn.c b/wpa_supplicant/ibss_rsn.c
>> index 53d7d57..c29d8d3 100644
>> --- a/wpa_supplicant/ibss_rsn.c
>> +++ b/wpa_supplicant/ibss_rsn.c
>> @@ -838,6 +838,18 @@ static void ibss_rsn_handle_auth_1_of_2(struct ibss_rsn *ibss_rsn,
>>             MAC2STR(addr));
>>
>>      if (peer &&
>> +        peer->authentication_status & (IBSS_RSN_SET_PTK_SUPP |
>> +                       IBSS_RSN_SET_PTK_AUTH)) {
>> +        /* Clear the TK for this pair to allow recovery from the case
>> +         * where the peer STA has restarted and lost its key while we
>> +         * still have a pairwise key configured. */
>> +        wpa_printf(MSG_DEBUG, "RSN: Clear pairwise key for peer "
>> +               MACSTR, MAC2STR(addr));
>> +        wpa_drv_set_key(ibss_rsn->wpa_s, WPA_ALG_NONE, addr, 0, 0,
>> +                NULL, 0, NULL, 0);
>> +    }
>> +
>> +    if (peer &&
>>          peer->authentication_status & IBSS_RSN_AUTH_EAPOL_BY_PEER) {
>>          if (peer->own_auth_tx.sec) {
>>              struct os_reltime now, diff;
>>
>
>
Ben Greear Jan. 19, 2017, 11:11 p.m. UTC | #4
After yet more testing, we found that IBSS mode works well when
the bssid is configured in wpa_supplicant, but it is much less reliable
if bssid is left blank.

I am not sure that is user-error or bug somewhere else, but
at this point, just configuring bssid is good enough for me.

I have tested on openwrt and LEDE successfully at this point.

Thanks,
Ben

On 01/17/2017 05:57 PM, Ben Greear wrote:
> I spent some more time debugging this today.
>
> When reloading the driver (and restarting supplicant), sometimes I can get two
> ath10k to talk IBSS + RSN, and others it fails.
>
> I instrumented the firmware to dump some state on demand, and it shows the same AST entries
> (where it stores pointers to the key data), and TIDs look similar as well
> in both the good and failure case.  So, it doesn't seem that the issue is
> lack of keys getting to the firmware.  Maybe the keys are wrong, but I am
> not sure how I could tell that.
>
> If I dump the actual key data for the peers, is that something I could
> compare against supplicant logs and verify the right keys are hitting
> the firmware?
>
> Is there anything I could do in supplicant to make the keys less random
> just to simplify testing (like make sure the hardware keys should always
> be the same value??)
>
> Thanks,
> Ben
>
> On 01/17/2017 01:26 PM, Ben Greear wrote:
>> On 01/14/2017 04:02 AM, Jouni Malinen wrote:
>>> On Fri, Jan 13, 2017 at 02:48:07PM -0800, Ben Greear wrote:
>>>> Ok, seems I should focus on events around Authentication message then.
>>>
>>> I was able to reproduce one related issue with mac80211_hwsim. This is a
>>> bug that prevents wpa_supplicant from clearing the key when receiving
>>> the Authentication frame from a peer. The patch below fixes that.
>>> However, this resulted in hitting another issue within mac80211 where it
>>> looks like the unicast frame from the re-joining STA gets lost in the RX
>>> reorder buffer logic.. I'm not completely sure what causes that, but for
>>> initial testing, using disable_ht=1 seemed to be a sufficient workaround
>>> (i.e., disable QoS and reorder buffer logic for BA).
>>
>> I added this patch, but my systems still do not work well.
>>
>> When I tried to put the stations in 'a' mode to disable ht, then the
>> association response is failing for some reason.
>>
>> Could you post an example of the config file you had that worked
>> for you?
>>
>> Thanks,
>> Ben
>>
>>
>>>
>>>
>>> [PATCH] RSN IBSS: Fix TK clearing on Authentication frame RX
>>>
>>> When wpa_supplicant was processing a received Authentication frame (seq
>>> 1) from a peer STA for which there was already a TK configured to the
>>> driver, debug log claimed that the PTK gets cleared, but the actual
>>> call to clear the key was actually dropped due to AUTH vs. SUPP set_key
>>> selection. Fix this by explicitly clearing the TK in case it was set
>>> and an Authentication frame (seq 1) is received.
>>>
>>> This fixes some cases where EAPOL-Key frames were sent encrypted using
>>> the old key when a peer STA restarted itself and lost the key and had to
>>> re-join the IBSS. Previously, that state required timing out the 4-way
>>> handshake and Deauthentication frame exchange to recover.
>>>
>>> Signed-off-by: Jouni Malinen <j@w1.fi>
>>> ---
>>>  wpa_supplicant/ibss_rsn.c | 12 ++++++++++++
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/wpa_supplicant/ibss_rsn.c b/wpa_supplicant/ibss_rsn.c
>>> index 53d7d57..c29d8d3 100644
>>> --- a/wpa_supplicant/ibss_rsn.c
>>> +++ b/wpa_supplicant/ibss_rsn.c
>>> @@ -838,6 +838,18 @@ static void ibss_rsn_handle_auth_1_of_2(struct ibss_rsn *ibss_rsn,
>>>             MAC2STR(addr));
>>>
>>>      if (peer &&
>>> +        peer->authentication_status & (IBSS_RSN_SET_PTK_SUPP |
>>> +                       IBSS_RSN_SET_PTK_AUTH)) {
>>> +        /* Clear the TK for this pair to allow recovery from the case
>>> +         * where the peer STA has restarted and lost its key while we
>>> +         * still have a pairwise key configured. */
>>> +        wpa_printf(MSG_DEBUG, "RSN: Clear pairwise key for peer "
>>> +               MACSTR, MAC2STR(addr));
>>> +        wpa_drv_set_key(ibss_rsn->wpa_s, WPA_ALG_NONE, addr, 0, 0,
>>> +                NULL, 0, NULL, 0);
>>> +    }
>>> +
>>> +    if (peer &&
>>>          peer->authentication_status & IBSS_RSN_AUTH_EAPOL_BY_PEER) {
>>>          if (peer->own_auth_tx.sec) {
>>>              struct os_reltime now, diff;
>>>
>>
>>
>
>
diff mbox

Patch

diff --git a/wpa_supplicant/ibss_rsn.c b/wpa_supplicant/ibss_rsn.c
index 53d7d57..c29d8d3 100644
--- a/wpa_supplicant/ibss_rsn.c
+++ b/wpa_supplicant/ibss_rsn.c
@@ -838,6 +838,18 @@  static void ibss_rsn_handle_auth_1_of_2(struct ibss_rsn *ibss_rsn,
 		   MAC2STR(addr));
 
 	if (peer &&
+	    peer->authentication_status & (IBSS_RSN_SET_PTK_SUPP |
+					   IBSS_RSN_SET_PTK_AUTH)) {
+		/* Clear the TK for this pair to allow recovery from the case
+		 * where the peer STA has restarted and lost its key while we
+		 * still have a pairwise key configured. */
+		wpa_printf(MSG_DEBUG, "RSN: Clear pairwise key for peer "
+			   MACSTR, MAC2STR(addr));
+		wpa_drv_set_key(ibss_rsn->wpa_s, WPA_ALG_NONE, addr, 0, 0,
+				NULL, 0, NULL, 0);
+	}
+
+	if (peer &&
 	    peer->authentication_status & IBSS_RSN_AUTH_EAPOL_BY_PEER) {
 		if (peer->own_auth_tx.sec) {
 			struct os_reltime now, diff;