[PATCH v6 06/17] wpa_supplicant: Set the correct key_type for key installs

Alexander Wetzel alexander at wetzel-home.de
Fri Sep 20 07:37:00 PDT 2019


Am 20.09.19 um 15:13 schrieb Jouni Malinen:
> On Sun, Sep 15, 2019 at 10:08:26PM +0200, Alexander Wetzel wrote:
>> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
>> @@ -200,7 +202,8 @@ int wpa_supplicant_set_wpa_none_key(struct wpa_supplicant *wpa_s,
>>   	/* TODO: should actually remember the previously used seq#, both for TX
>>   	 * and RX from each STA.. */
>>   
>> -	ret = wpa_drv_set_key(wpa_s, alg, NULL, 0, 1, seq, 6, key, keylen, 0);
>> +	ret = wpa_drv_set_key(wpa_s, alg, NULL, 0, 1, seq, 6, key, keylen,
>> +			      KEY_TYPE_BROADCAST);
> 
> Is this really KEY_TYPE_BROADCAST instead of KEY_TYPE_DEFAULT? As noted
> in the beginning of this function, only one key is used for both
> receiving and sending unicast and multicast frames.

We are deleting a key. KEY_TYPE_DEFAULT is basically just used to make a 
broadcast key also to the default key. (Kind like an upgrade...)

But a deleted key can't be used for default... I tried to document that 
KEY_TYPE_DEFAULT is not valid for key deletions in the key_type 
comments. Can't rule out that we have to update any other section when 
starting to use KEY_TYPE_DEFAULT but it could even work as it is, I think.

> 
>> diff --git a/wpa_supplicant/wpas_glue.c b/wpa_supplicant/wpas_glue.c
>> @@ -341,7 +342,7 @@ static void wpa_supplicant_eapol_cb(struct eapol_sm *eapol,
>>   			"handshake", pmk, pmk_len);
>>   
>>   	if (wpa_drv_set_key(wpa_s, WPA_ALG_PMK, NULL, 0, 0, NULL, 0, pmk,
>> -			    pmk_len, 0)) {
>> +			    pmk_len, KEY_TYPE_BROADCAST)) {
> 
> WPA_ALG_PMK is not for a cipher, it is for offloading 4-way handshake to
> the driver. As such, KEY_TYPE_BROADCAST looks strange here. Maybe we
> should have KEY_TYPE_OTHER (etc.) for this special case(?)
> 

You are right. I catched that in wpa_supplicant_key_mgmt_set_pmk() but 
missed it in the code above.
For wpa_supplicant_key_mgmt_set_pmk() I was also thinking about adding 
an additional key type but in the end just decided to use "0" instead of 
"KEY_TYPE_BROADCAST" - which of course also is 0. KEY_TYPE_OTHER or - we 
have more than enough space in the enum - even a dedicated KEY_TYPE_PMK 
- are the other options and any of them are fine for me.

Alexander



More information about the Hostap mailing list