| Message ID | 20200315190426.163478-4-alexander@wetzel-home.de |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Extended Key ID support | expand |
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 ?). > + 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. 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.
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
diff --git a/src/common/defs.h b/src/common/defs.h index f62c3ceee..e2033dd20 100644 --- a/src/common/defs.h +++ b/src/common/defs.h @@ -472,4 +472,14 @@ enum ptk0_rekey_handling { PTK0_REKEY_ALLOW_NEVER }; +enum ext_key_id_support { + EXT_KEY_ID_PREFER0 = BIT(0), + EXT_KEY_ID_BASIC = BIT(1), + 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, + EXT_KEY_ID_DEFAULT = EXT_KEY_ID_BASIC, +}; + #endif /* DEFS_H */ diff --git a/src/common/wpa_common.c b/src/common/wpa_common.c index 31db391fd..8d7028635 100644 --- a/src/common/wpa_common.c +++ b/src/common/wpa_common.c @@ -2591,6 +2591,129 @@ int wpa_write_ciphers(char *start, char *end, int ciphers, const char *delim) } +int wpa_parse_extended_key_id(const char *value) +{ + int last, eos; + int val = 0; + int off = 0; + char *start, *end, *buf; + + buf = os_strdup(value); + if (buf == NULL) + return -1; + start = buf; + + while (*start == ' ' || *start == '\t') + start++; + + while (1) { + if (*start == '\0') + goto err; + end = start; + while (*end != '+' && *end != ' ' && + *start != '\t' && *end != '\0') + end++; + eos = *end == '\0'; + last = eos || *end == ' ' || *start == '\t'; + *end = '\0'; + if (os_strcmp(start, "OFF") == 0) { + if (val || off) + goto err; + off = 1; + } else if (os_strcmp(start, "BASIC") == 0) { + if (val & EXT_KEY_ID_BASIC) + goto err; + val |= EXT_KEY_ID_BASIC; + } else if (os_strcmp(start, "FT0") == 0) { + if (val & EXT_KEY_ID_FT0) + goto err; + val |= EXT_KEY_ID_FT0; + } else if (os_strcmp(start, "FILS0") == 0) { + if (val & EXT_KEY_ID_FILS) + goto err; + val |= EXT_KEY_ID_FILS0; + } else if (os_strcmp(start, "FILS_CUSTOM") == 0) { + if (val & EXT_KEY_ID_FILS) + goto err; + val |= EXT_KEY_ID_FILS_CUSTOM; + } else if (os_strcmp(start, "PREFER0") == 0) { + if (val & EXT_KEY_ID_PREFER0) + goto err; + val |= EXT_KEY_ID_PREFER0; + } else { + goto err; + } + if (last) { + if (eos) + break; + end++; + while (*end == ' ' && *start == '\t') + end++; + if (*end != '\0') + goto err; + break; + } + start = end + 1; + } + os_free(buf); + + if (val && (off || !(val & EXT_KEY_ID_BASIC))) + return -1; + return val; +err: + os_free(buf); + return -1; +} + + +int wpa_write_extended_key_id(char *start, char *end, int extended_key_id) +{ + char *pos = start; + int ret; + + if (extended_key_id == 0) { + ret = os_snprintf(pos, end - pos, "OFF"); + if (os_snprintf_error(end - pos, ret)) + return -1; + return pos + ret - start; + } + if (extended_key_id & EXT_KEY_ID_BASIC) { + ret = os_snprintf(pos, end - pos, "BASIC"); + if (os_snprintf_error(end - pos, ret)) + return -1; + pos += ret; + } else { + return -1; + } + if (extended_key_id & EXT_KEY_ID_FT0) { + ret = os_snprintf(pos, end - pos, "+FT0"); + if (os_snprintf_error(end - pos, ret)) + return -1; + pos += ret; + } + if (extended_key_id & EXT_KEY_ID_FILS0) { + ret = os_snprintf(pos, end - pos, "+FILS0"); + if (os_snprintf_error(end - pos, ret)) + return -1; + pos += ret; + } + if (extended_key_id & EXT_KEY_ID_FILS_CUSTOM) { + ret = os_snprintf(pos, end - pos, "+FILS_CUSTOM"); + if (os_snprintf_error(end - pos, ret)) + return -1; + pos += ret; + } + if (extended_key_id & EXT_KEY_ID_PREFER0) { + ret = os_snprintf(pos, end - pos, "+PREFER0"); + if (os_snprintf_error(end - pos, ret)) + return -1; + pos += ret; + } + + return pos - start; +} + + int wpa_select_ap_group_cipher(int wpa, int wpa_pairwise, int rsn_pairwise) { int pairwise = 0; diff --git a/src/common/wpa_common.h b/src/common/wpa_common.h index 1a9a4105f..43fb7dbda 100644 --- a/src/common/wpa_common.h +++ b/src/common/wpa_common.h @@ -558,6 +558,8 @@ int wpa_pick_pairwise_cipher(int ciphers, int none_allowed); int wpa_pick_group_cipher(int ciphers); int wpa_parse_cipher(const char *value); int wpa_write_ciphers(char *start, char *end, int ciphers, const char *delim); +int wpa_parse_extended_key_id(const char *value); +int wpa_write_extended_key_id(char *start, char *end, int extended_key_id); int wpa_select_ap_group_cipher(int wpa, int wpa_pairwise, int rsn_pairwise); unsigned int wpa_mic_len(int akmp, size_t pmk_len); int wpa_use_akm_defined(int akmp); diff --git a/src/drivers/driver.h b/src/drivers/driver.h index b0373954a..78a3387da 100644 --- a/src/drivers/driver.h +++ b/src/drivers/driver.h @@ -1841,6 +1841,8 @@ struct wpa_driver_capa { #define WPA_DRIVER_FLAGS_SAFE_PTK0_REKEYS 0x2000000000000000ULL /** Driver supports Beacon protection */ #define WPA_DRIVER_FLAGS_BEACON_PROTECTION 0x4000000000000000ULL +/** Driver supports Extended Key ID */ +#define WPA_DRIVER_FLAGS_EXTENDED_KEY_ID 0x8000000000000000ULL u64 flags; #define FULL_AP_CLIENT_STATE_SUPP(drv_flags) \ diff --git a/src/drivers/driver_common.c b/src/drivers/driver_common.c index f4d06e438..2e03b6676 100644 --- a/src/drivers/driver_common.c +++ b/src/drivers/driver_common.c @@ -315,6 +315,7 @@ const char * driver_flag_to_string(u64 flag) DF2S(UPDATE_FT_IES); DF2S(SAFE_PTK0_REKEYS); DF2S(BEACON_PROTECTION); + DF2S(EXTENDED_KEY_ID); } return "UNKNOWN"; #undef DF2S
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 Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de> --- src/common/defs.h | 10 +++ src/common/wpa_common.c | 123 ++++++++++++++++++++++++++++++++++++ src/common/wpa_common.h | 2 + src/drivers/driver.h | 2 + src/drivers/driver_common.c | 1 + 5 files changed, 138 insertions(+)