Question on setting key right after the EAPOL 4/4 is sent.

[Janusz Dziedzic]

2017-06-09 22:18 GMT+02:00 Ben Greear <greearb at candelatech.com>:
> On 06/09/2017 01:01 PM, Johannes Berg wrote:
>>
>> On Fri, 2017-06-09 at 06:46 -0700, Ben Greear wrote:
>>
>>>> However, the solution is far simpler! Once you have nl80211 PAE
>>>> transport, you can easily even set the key before transmitting the
>>>> packet and simply indicate that this particular packet should _not_
>>>> be encrypted regardless of key presence.
>>>
>>>
>>> My ath10k firmware cannot deal with a case like this:
>>>
>>> pkt is enqueued before key is set
>>> key is set
>>> pkt is transmitted (incorrectly)
>>>
>>> This is because of how the tid's header-length variables are set up
>>> and modified when the keys are set, and I don't see any good way to
>>> fix this.
>>
>>
>> That seems awful, and anyway will not work with the mentioned non-IEEE
>> protocols that require not encrypting the rekeying frames even when
>> keys have been set up.
>>
>> I don't know what to tell you here, I think it'd be best if you fix
>> that.
>
>
> The case that fails is basically any packet that is currently
> enqueued in the firmware when the key is set is transmitted incorrectly or
> not at all.
> And, maybe this is only for DATA tids.
>
> Otherwise, the no-encrypt logic should work fine.  So, it is just the race
> with the EAPOL 4/4 and set-key that causes issues as far as I can tell.
>
> It looks like the EAPOL 4/4 and set-key race is fixable without too much
> effort,
> so I think I will focus on that for now instead of further hacking special
> case logic into the firmware.
>
Ben, as I remember correctly there is a flag,
WMI_PEER_NEED_PTK_4_WAY

This fix race (you describe) in the firmware/hw.
For 636 firmware, where we tested STA mode really hard (5 years ago),
that works perfect.
So, I am not sure why this flag don't work correctly with your firmware.

Regarding EAPOL frames I am not sure you will have real/correct ACK. I
remember in case of mgmt frames FW always return ACK - but probably
you will check that.
I also fight with some EAPOL frames, but forgot why ... Seems my brain
removed this information for some reason :-)

BR
Janusz

>>> Stock ath10k firmware goes to great lengths to parse EAPOL frames and
>>> try to work around it in that manner, but that breaks .11r (or used
>>> to, I haven't tried stock firmware lately) and adds more complexity
>>> to the code.
>>
>>
>> It just has to be a single flag saying "don't encrypt this frame" -
>> nothing super complicated about that?
>>
>> In ath10k it looks like HTT_DATA_TX_DESC_FLAGS0_NO_ENCRYPT gets set for
>> this, seems easy enough?
>>
>>>  From a patch someone sent to hostapd list last night, it seems we
>>> could get the tx-status for the EAPOL 4/4, and in that case, we
>>> *know* the pkt has been transmitted, so we can then set the key
>>> safely it would seem?
>>
>>
>> I think so, and I don't remember why we dismissed this solution. Could
>> be that we just decided solving the bridging issue at the same time,
>> while not introducing more latency, was better.
>>
>> Also, the other way can possibly solve some PTK rekeying issues, so
>> overall the solution to go all the way seems better.
>
>
> I guess I don't fully understand the PAE thing, but if you all like it,
> then sounds good to me.
>
> For kernels not supporting this new feature, it seems that having supplicant
> wait for tx-status would be a good interim fix?
>
> Thanks,
> Ben
>
> --
> Ben Greear <greearb at candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com
>
>
> _______________________________________________
> Hostap mailing list
> Hostap at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/hostap



-- 
Janusz Dziedzic