From:   Alexander Wetzel <alexander@wetzel-home.de>
Subject: Re: [PATCH] mac80211: Honor SW_CRYPTO_CONTROL in AP VLAN mode
To:     Johannes Berg <johannes@sipsolutions.net>
Cc:     linux-wireless@vger.kernel.org, mpubbise@codeaurora.org,
        Christian.Limpach@gmail.com
References: <20190209140138.16692-1-alexander@wetzel-home.de>
 <ad27a4098892dbd2ebd1954ba6f34ae0fea4600c.camel@sipsolutions.net>
Message-ID: <d7f567e6-3467-451a-8bd3-7819688e779f@wetzel-home.de>
Date:   Mon, 8 Apr 2019 21:58:18 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <ad27a4098892dbd2ebd1954ba6f34ae0fea4600c.camel@sipsolutions.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

>> Restore @SW_CRYPTO_CONTROL when interface is in AP Vlan mode and don't
>> override driver decision for unicast keys.
>>
>> Fixes commit db3bdcb9c3ff ("mac80211: allow AP_VLAN operation on crypto
>> controlled devices"), which should only have allow SW crypto fallback for
>> group keys.
> 
> This confuses me.
> 
> The driver doesn't really know about AP_VLAN, so the original commit
> intentionally did this, I think?
> 
> There was some kind of other fix related to this though?
Here how I understand the situation:

The intent of db3bdcb9c3ff ("mac80211: allow AP_VLAN operation on crypto 
controlled devices") was, to allow devices setting SW_CRYPTO_CONTROL to 
still support AP_VLAN group keys. It unintentionally allowed more than 
it should for that...

Prior to the commit db3bdcb9c3ff installing a group key when in AP_VLAN 
mode could not work. Mac80211 was initializing ret to -EOPNOTSUPP and 
then jumped to "out_unsupported" when the interface was in AP_VLAN mode 
without trying to install the key and giving the driver a chance to 
allow SW crypto. Software fallback was then blocked by ret != 1 with 
SW_CRYPTO_CONTROL set by the driver.

But any driver setting NL80211_IFTYPE_AP_VLAN already has confirmed to 
be fine with SW crypto for group keys, since these CAN only be handled 
with SW crypto currently. The net effect was that ath10k - the only 
driver in tree setting SW_CRYPTO_CONTROL and also NL80211_IFTYPE_AP - 
was not able to send out frames encrypted with the group key at all.

Now the fix for that was commit db3bdcb9c3ff. Unfortunately this went 
too far:
We now not only allows fallback to SW crypto for group keys (which the 
driver allowed by setting NL80211_IFTYPE_AP_VLAN) but for ANY keys as 
long as the interface is in AP_VLAN mode.
So if for some reasons ath10k is not able to install a pairwise key 
mac80211 now incorrectly allows the fallback to SW crypto...

This patch here tries to fix that and excludes pairwise keys from the 
special AP_VLAN handling again (but keeps it for group keys).

This understanding is based on the result of a inquiry to linux-wireless 
(see https://patchwork.kernel.org/patch/10313127/) and a short 
out-of-list discussion with Christian Limpach who pointed me to 
http://lists.infradead.org/pipermail/ath10k/2018-November/012542.html
as the reason for the original fix in db3bdcb9c3ff.

Alexander