[v6,09/17] nl80211,wpa_supplicant: Drop outdated tdls hack
diff mbox series

Message ID 20190915200837.196283-10-alexander@wetzel-home.de
State New
Headers show
Series
  • Support seamless PTK rekeys with Extended Key ID
Related show

Commit Message

Alexander Wetzel Sept. 15, 2019, 8:08 p.m. UTC
wpa_tdls_set_key() did set key_id to -1 as a signal to handle the key
install a bit different than for other pairwise keys.

Since we cleaned up the key install logic with a previous patch this is
no longer needed and can be removed.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---

The workaround cleaned up here is not limited to nl80211, so I put it
into a separate patch. It just finalizes the nl80211 driver key install
cleanup without breaking anything in between.

 src/drivers/driver_nl80211.c | 6 ------
 src/rsn_supp/tdls.c          | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)

Comments

Jouni Malinen Sept. 20, 2019, 10:03 a.m. UTC | #1
On Sun, Sep 15, 2019 at 10:08:29PM +0200, Alexander Wetzel wrote:
> wpa_tdls_set_key() did set key_id to -1 as a signal to handle the key
> install a bit different than for other pairwise keys.
> 
> Since we cleaned up the key install logic with a previous patch this is
> no longer needed and can be removed.
> 
> Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
> ---
> 
> The workaround cleaned up here is not limited to nl80211, so I put it
> into a separate patch. It just finalizes the nl80211 driver key install
> cleanup without breaking anything in between.

This key_id == -1 case is used by an old out-of-tree driver wrapper to
make TDLS work. In other words, this commit would break that. Not that
I'm too worried about out-of-tree code, but I'd like to understand what
exactly this patch is trying to achieve. Is this just cleanup and as
such, could it be dropped without breaking anything in this patch
series?
Alexander Wetzel Sept. 20, 2019, 10:53 a.m. UTC | #2
Am 20.09.19 um 12:03 schrieb Jouni Malinen:
> On Sun, Sep 15, 2019 at 10:08:29PM +0200, Alexander Wetzel wrote:
>> wpa_tdls_set_key() did set key_id to -1 as a signal to handle the key
>> install a bit different than for other pairwise keys.
>>
>> Since we cleaned up the key install logic with a previous patch this is
>> no longer needed and can be removed.
>>
>> Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
>> ---
>>
>> The workaround cleaned up here is not limited to nl80211, so I put it
>> into a separate patch. It just finalizes the nl80211 driver key install
>> cleanup without breaking anything in between.
> 
> This key_id == -1 case is used by an old out-of-tree driver wrapper to
> make TDLS work. In other words, this commit would break that. Not that
> I'm too worried about out-of-tree code, but I'd like to understand what
> exactly this patch is trying to achieve. Is this just cleanup and as
> such, could it be dropped without breaking anything in this patch
> series?

Yes, this is basically only a cleanup.

When we want to drop the patch it it won't have any real consequences 
for the rest of the series.

It only would get in the way when we decide to also use unicast keyid 1 
for TDLS, too. But the standard has no guidance on that and TDLS isn't 
caring about rekeys much... (I looked into that a few hours and then 
basically dropped any plans to use Extended Key ID also for TDLS. We 
would first to have define something like "Extended-TDLS" and when 
rekeys are not an issue in TDLS why bother?)


Alexaner

Patch
diff mbox series

diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index f168769f8..f15f1e15f 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -3027,12 +3027,6 @@  static int wpa_driver_nl80211_set_key(const char *ifname, struct i802_bss *bss,
 		   "set_tx=%d seq_len=%lu key_len=%lu key_type=%d",
 		   __func__, ifindex, ifname, alg, addr, key_idx, set_tx,
 		   (unsigned long) seq_len, (unsigned long) key_len, key_type);
-#ifdef CONFIG_TDLS
-	if (key_idx == -1) {
-		key_idx = 0;
-	}
-#endif /* CONFIG_TDLS */
-
 #ifdef CONFIG_DRIVER_NL80211_QCA
 	if (alg == WPA_ALG_PMK &&
 	    (drv->capa.flags & WPA_DRIVER_FLAGS_KEY_MGMT_OFFLOAD)) {
diff --git a/src/rsn_supp/tdls.c b/src/rsn_supp/tdls.c
index 348c491be..01d339290 100644
--- a/src/rsn_supp/tdls.c
+++ b/src/rsn_supp/tdls.c
@@ -227,7 +227,7 @@  static int wpa_tdls_set_key(struct wpa_sm *sm, struct wpa_tdls_peer *peer)
 
 	wpa_printf(MSG_DEBUG, "TDLS: Configure pairwise key for peer " MACSTR,
 		   MAC2STR(peer->addr));
-	if (wpa_sm_set_key(sm, alg, peer->addr, -1, 1, rsc, sizeof(rsc),
+	if (wpa_sm_set_key(sm, alg, peer->addr, 0, 1, rsc, sizeof(rsc),
 			   peer->tpk.tk, key_len, KEY_TYPE_PAIRWISE) < 0) {
 		wpa_printf(MSG_WARNING, "TDLS: Failed to set TPK to the "
 			   "driver");