Re: [PATCH 3/8] common: Extended Key ID support

Am 15.03.20 um 22:59 schrieb Jouni Malinen:
> On Sun, Mar 15, 2020 at 08:04:21PM +0100, Alexander Wetzel wrote:
>> Add shared functions and variables for Extended Key ID support:
>>
>>   - Add "enum ext_key_id_support" for config options
>>   - Add helper functions to read/write Extended Key ID config options
>>   - Add the new driver flag WPA_DRIVER_FLAGS_EXTENDED_KEY_ID
> 
> I'll split that last part to go in with the nl80211 changes so that this
> patch 3/8 is only for introducing configuration definitions and helpers.
> 
>> diff --git a/src/common/defs.h b/src/common/defs.h
>> +enum ext_key_id_support {
>> +	EXT_KEY_ID_PREFER0	= BIT(0),
> 
> PREFER0 sounds like a pretty confusing name. Isn't this for specifying
> which KeyID to use initially instead of somehow preferring 0 over 1 in
> general. Furthermore, I'm not going to force interop testing to users by
> default, so Key ID 0 needs to be the default approach and there can be
> optional configuration mechanism for starting with 1 (+START1 ?).
> 

I did not plan that to be for interop testing. The idea was to make sure 
that when there are interop issues they are glaring obvious instead of 
random. But I see you don't agree and will simply drop it.

>> +	EXT_KEY_ID_BASIC	= BIT(1),
> 
> Similarly, "BASIC" sounds confusing.. Isn't this what would be done to
> simply enable Extended Key ID?
> 
>> +	EXT_KEY_ID_FT0		= BIT(2),
>> +	EXT_KEY_ID_FILS0	= BIT(3),
>> +	EXT_KEY_ID_FILS_CUSTOM	= BIT(4),
>> +	EXT_KEY_ID_FILS		= EXT_KEY_ID_FILS0 | EXT_KEY_ID_FILS_CUSTOM,
> 
> And these get really confusing.. I don't think I want to include any
> custom stuff or protocol extensions that do not match what is defined in
> the standard. In other words, I'd expect there to be configuration for
> enabling Extended Key ID with an option to enable testing mode that
> starts with KeyID 1. For example:
> 
> extended_key_id=<0/1/2>
> 0: disabled
> 1: enabled
> 2: enabled with test mode to start with KeyID 1
> 
> FILS and FT cases could need to be managed automatically. I'm not sure
> why this would need any more complexity. The AP can advertise support
> for Extended Key ID but only issue keys with KeyID 1 for the cases that
> are fully defined in the standard.
> 
BASIC is just what I'm 100% sure is in the standard. You seem to read it 
a bit differently and also see what I called FT0 and FILS0 to be 
standardized. You just seem to prefer to also allow switching "late" to 
Extended Key ID and not force the decision at the initial connect.

I'm currently enforcing both STA and AP stick to the initial decision to 
prevent a STA to e.g. connect with FT, rekey once with PTK0 but then set 
the Extended Key ID bit to use it once.

> It's fine to have proposed patches ready for likely extensions of FT and
> FILS, but I'd like to see those as separate patches so that the fully
> defined functionality can first be applied without any such changes
> included.
> 

As long as I did not misunderstand you or the consequences of of what 
you are asking for there seems to be no need for additional patches.
When we allow FT and FILS to be used and use keyid 0 for those 
handshakes we should be set.

And sounds like I should relax the sanity checks allowing to switch more 
or less any time to extended Key ID.


Alexander