Re: wpa_supplicant 2.8 fails in brcmf_cfg80211_set_pmk

[Arend Van Spriel <arend.vanspriel@broadcom.com>]

On 6/19/2019 7:26 AM, Marcel Holtmann wrote:
> Hi Arend,
> 
>>>>>>> i was able to reproduce an (maybe older issue) with 4-way handshake
>>>>>>> offloading for 802.1X in the brcmfmac driver. My setup consists of
>>>>>>> Raspberry Pi 3 B (current linux-next, arm64/defconfig) on STA side and a
>>>>>>> Raspberry Pi 3 A+ (Linux 4.19) on AP side.
>>>>>>
>>>>>> Looks like Raspberry Pi isn't the only affected platform [3], [4].
>>>>>>
>>>>>> [3] - https://bugzilla.redhat.com/show_bug.cgi?id=1665608
>>>>>> [4] - https://bugzilla.kernel.org/show_bug.cgi?id=202521
>>>>>
>>>>> Stefan,
>>>>>
>>>>> Could you please try the attached patch for your wpa_supplicant? We'll
>>>>> upstream if it works for you.
>>>>
>>>> I hope that someone is also providing a kernel patch to fix the issue. Hacking around a kernel issue in userspace is not enough. Fix the root cause in the kernel.
>>> Marcel,
>>> This is a kernel warning for invalid application PMK set actions, so the
>>> fix is to only set PMK to wifi driver when 4-way is offloaded. I think
>>> Arend added the WARN_ON() intentionally to catch application misuse of
>>> PMK setting.
>>> You may also remove the warnings with the attached patch, but let's see
>>> what Arend says first.
>>> Arend,
>>> Any comment?
>>
>> Hi Chi-Hsien, Marcel
>>
>>  From the kernel side I do not see an issue. In order to use 802.1X offload the NL80211_ATTR_WANT_1X_4WAY_HS flag must be set in NL80211_CMD_CONNECT. Otherwise, NL80211_CMD_SET_PMK is not accepted. The only improvement would be to document this more clearly in the "WPA/WPA2 EAPOL handshake offload" DOC section in nl80211.h.
> 
> so nl80211 is an API. And an application can use that API wrongly (be that intentionally or unintentionally), the kernel can not just go WARN_ON and print a backtrace. That is your bug. So please handle wrong user input properly.

Hi Marcel,

You are right. However, the kernel does also return an error if the 
WARN_ON is hit. We can improve by using the EXT_ACK functionality to 
provide more info than just -EINVAL, eg. "PMK not accepted; no 802.1X 
offload requested on connect".

> Frankly, I don’t get why nl80211 itself is not validating the input and this is left to the driver. I think we need a nl80211 fuzzer that really exercises this API with random values and parameters to provide invalid input.

That would mean nl80211 should keep state info between commands. From 
what I remember that has been avoided from day one because of the 
experiences with that in the WEXT days. I welcome any testing be it 
fuzzer or something else.

Regards,
Arend