Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] iwlwifi: add NL80211_EXT_FEATURE_CAN_REPLACE_PTK0 support
To:     Johannes Berg <johannes@sipsolutions.net>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>
Cc:     "luciano.coelho@intel.com" <luciano.coelho@intel.com>,
        "emmanuel.grumbach@intel.com" <emmanuel.grumbach@intel.com>,
        "linuxwifi@intel.com" <linuxwifi@intel.com>
References: <20200918171301.6942-1-alexander@wetzel-home.de>
 <f6df593556c3f395997dfe42a71a69f4919a5911.camel@sipsolutions.net>
From:   Alexander Wetzel <alexander@wetzel-home.de>
Message-ID: <c887ae58-983b-0f4b-3b00-8ffbb669f37a@wetzel-home.de>
Date:   Sun, 20 Sep 2020 08:37:15 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.12.0
MIME-Version: 1.0
In-Reply-To: <f6df593556c3f395997dfe42a71a69f4919a5911.camel@sipsolutions.net>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


 >>
 >> +		/* GCMP and 256 bit CCMP keys the key can't be copied into the
 >> +		 * MPDU struct ieee80211_tx_info. We therefore must flush the
 >> +		 * queues to ensure there are no MPDUs left which are referring
 >> +		 * to the outgoing key.
 >> +		 */
 >> +		if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE &&
 >> +		    (key->cipher == WLAN_CIPHER_SUITE_GCMP ||
 >> +		     key->cipher == WLAN_CIPHER_SUITE_GCMP_256 ||
 >> +		     key->cipher == WLAN_CIPHER_SUITE_CCMP_256)) {
 >> +			ieee80211_stop_queues(hw);
 >> +			iwl_mvm_mac_flush(hw, vif, 0, true);
 >> +			ieee80211_wake_queues(hw);
 >> +		}
 >
 > Shouldn't the wake be after installing the new key? Otherwise new frames
 > can race and be there again?

mac80211 is taking care of that and has already stopped queuing new 
MPDUs which would use the key by setting KEY_FLAG_TAINTED on it. So for 
a PTK0 rekey we are fine, mac80211 won't queue frames with the key under 
deletion.

But I think we should start setting KEY_FLAG_TAINTED for *any* PTK 
deletion to make sure we are not sending out MPDUs with a zero key or 
something like that. This is a special kind of a rekey case we missed so 
far but should be trival cover that, too.

That is then basically taking care about the generic Kr00k vulnerability 
  some months ago.

Now I'm not aware of we have any mitigations in the code for that but 
when we set KEY_FLAG_TAINTED for key deletions any driver which is able 
to rekey PTK0 correctly can't be affected by Kr00k any longer.

I'll look into that in the next days and prepare a patch for mac8011 to 
discuss that.

Alexander