netdev - Re: [PATCH] rsi: fix broken AP mode due to unwanted encryption

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c503e8e8-6082-3f81-f22d-efdbf4a1b357@denx.de>
Date:   Tue, 1 Jun 2021 13:24:09 +0200
From:   Marek Vasut <marex@...x.de>
To:     "Fuzzey, Martin" <martin.fuzzey@...wbird.group>
Cc:     Amitkumar Karwar <amitkarwar@...il.com>,
        Siva Rebbagondla <siva8118@...il.com>,
        Kalle Valo <kvalo@...eaurora.org>,
        linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
        "Linux-Kernel@...r. Kernel. Org" <linux-kernel@...r.kernel.org>,
        Angus Ainslie <angus@...ea.ca>,
        Guido Günther <agx@...xcpu.org>
Subject: Re: [PATCH] rsi: fix broken AP mode due to unwanted encryption

On 6/1/21 11:02 AM, Fuzzey, Martin wrote:
> Hi Marek,
> thanks for the review.

Hi,

> On Fri, 28 May 2021 at 20:11, Marek Vasut <marex@...x.de> wrote:
>>
>>> Signed-off-by: Martin Fuzzey <martin.fuzzey@...wbird.group>
>>> CC: stable@...r.kernel.org
>>
>> This likely needs a Fixes: tag ?
>>
> 
> I'm not quite sure what that should be.

Based on git log -p --follow drivers/net/wireless/rsi/rsi_91x_hal.c

Fixes: dad0d04fa7ba ("rsi: Add RS9113 wireless driver")

> The test involved here has been present since the very first version
> of the driver back in 2014 but at that time AP mode wasn't supported.
> 
> AP mode was added in 2017 by the patch series "rsi: support for AP mode" [1]
> In particular 38ef62353acb ("rsi: security enhancements for AP mode")
> does some stuff relating to AP key configuration but it doesn't
> actually change the behaviour concerning the encryption condition.
> 
> In fact I don't understand how it ever worked in AP WPA2 mode given
> that secinfo->security_enable (which is tested in the encryption
> condition) has always been unconditionally set in set_key (when
> setting not deleting).
> Yet the series cover letter [1] says "Tests are performed to confirm
> aggregation, connections in WEP and WPA/WPA2 security."

There are way too many bugs in that RSI driver, yes. Compared to the 
other WiFi vendors, this driver seems to be real poor.

> The problem is that in AP mode with WPA2 there is a set_key done at AP
> startup time to set the GTK (but not yet the pairwise key which is
> only done after the 4 way handshake) so this sets security_enable to
> true which later causes the EAPOL messages to be sent encrypted.
> 
> Maybe there have been userspace changes to hostapd that have changed
> the time at which the GTK is set?
> I had a quick look at the hostapd history but didn't see anything obvious.

I recall seeing a patched WPA supplicant in one set of the RSI 
downstream drivers. And then there is another older RSI "onebox" driver 
which definitely ships patched userspace components. So I wouldn't be 
surprised if some of that was involved.

> I'm going to send a V2 completely removing the security_enable flag in
> addition to adding the new test (which is what downstream has too).
> Keeping security_enable doesn't actually break anything but is
> redundant and confusing.
> 
> Unfortunately I cannot find any downstream history, I just have 2
> downstream tarballs, a "2.0 RC2" which has the same problem as
> mainline and a "2.0 RC4" which does not

Yes indeed, I pointed that out to RSI before, that this kind of release 
management is useless. I'm still waiting for any input from them.

> [1] https://www.spinics.net/lists/linux-wireless/msg165302.html
> 
>>>        if ((!(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT)) &&
>>> +         (info->control.hw_key) &&
>>
>> The () are not needed.
>>
> 
> Ok, will fix for V2

Thanks