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

[Ben Greear]

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.

>> 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