diff mbox series

[2/3] AP: Drop not needed condition to delete PTK ID 1

Message ID 20200323184228.10798-2-alexander@wetzel-home.de
State Accepted
Headers show
Series [1/3] AP: Fix Extended Key ID parameter check | expand

Commit Message

Alexander Wetzel March 23, 2020, 6:42 p.m. UTC 
Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---

Here my code was also broken in the last patch. We can probably keep it
as it is now but I wanted to comment on that:

The intent seems to be to make sure all keys have been removed to reset
to a defined state. (At least we are unconditional deleting PTK ID 0.)

If that's correct using the "current" settings here is wrong and we should
always delete PTK ID 1 when the driver can handle Extended Key ID.
And for that perspective the patch here is still wrong and no
improvement:
I do not see an easy way to access the driver flags here I therefore just
"simplified" the statement. After all use_ext_key_id can only be
true when extended_key_id has been enabled in the config.

We also could just unconditionally try to delete PTK ID 1 but then we
have an error in the logs..

 src/ap/wpa_auth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jouni Malinen March 29, 2020, 7:46 p.m. UTC | #1 
On Mon, Mar 23, 2020 at 07:42:27PM +0100, Alexander Wetzel wrote:
> Here my code was also broken in the last patch. We can probably keep it
> as it is now but I wanted to comment on that:

Thanks, applied.

> The intent seems to be to make sure all keys have been removed to reset
> to a defined state. (At least we are unconditional deleting PTK ID 0.)
> 
> If that's correct using the "current" settings here is wrong and we should
> always delete PTK ID 1 when the driver can handle Extended Key ID.
> And for that perspective the patch here is still wrong and no
> improvement:
> I do not see an easy way to access the driver flags here I therefore just
> "simplified" the statement. After all use_ext_key_id can only be
> true when extended_key_id has been enabled in the config.
> 
> We also could just unconditionally try to delete PTK ID 1 but then we
> have an error in the logs..

This should not really matter at all with nl80211 and that's currently
the only driver interface supporting a pairwise key with Key ID 1, so
this should be fine for the time being. In fact, all those key clearing
operations could be removed whenever nl80211 is used as the driver
interface. Some other driver interfaces may need explicit clearing,
though, which is the reason why these were added in the first place and
have then been extended to cover new cases.
diff mbox series

Patch

diff --git a/src/ap/wpa_auth.c b/src/ap/wpa_auth.c
index 6512c0194..bd522f669 100644
--- a/src/ap/wpa_auth.c
+++ b/src/ap/wpa_auth.c
@@ -1756,7 +1756,7 @@  void wpa_remove_ptk(struct wpa_state_machine *sm)
 			     0, KEY_FLAG_PAIRWISE))
 		wpa_printf(MSG_DEBUG,
 			   "RSN: PTK removal from the driver failed");
-	if (sm->wpa_auth->conf.extended_key_id && sm->use_ext_key_id &&
+	if (sm->use_ext_key_id &&
 	    wpa_auth_set_key(sm->wpa_auth, 0, WPA_ALG_NONE, sm->addr, 1, NULL,
 			     0, KEY_FLAG_PAIRWISE))
 		wpa_printf(MSG_DEBUG,