| Message ID | 20210716154521.2128892-1-jefferymiller@google.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Avoid PMF negotiation for networks if the driver does not support PMF | expand |
On Fri, Jul 16, 2021 at 03:45:21PM +0000, Jeffery Miller wrote: > Networks configured with ieee80211w=1 will fail to connect > to a PMF enabled AP during negotiation if the driver does > not support PMF. > Extend the existing global driver PMF capability check to > apply when the network specific ieee80211w configuration > value is set to optional. > This allows networks configured with PMF as optional to > make use of this existing driver check. What's the use case for this change? It was more justifiable to do this for the newer global pmf=1 case, but I'm a bit hesitant on changing the more explicit network block ieee80211w=1 behavior since it would break number of currently working cases. The main issue here is in many drivers supporting PMF without explicitly indicating support for BIP. As an example, this patch would break PMF optional case with any other driver interface than nl80211. I'm not completely sure about the nl80211 cases since the BIP cipher suite support indication might have been added later than the initial PMF implementation. This may have resulted in there being no strict rejection of BIP configuration with drivers that do not have explicit indication for it in the supported ciphers list. As such, it may be a bit difficult to do this type of a change in wpa_supplicant without the kernel interface(s) changing first to explicitly indicate whether PMF is supported. At minimum, this would need something like the following, but I'm not yet convinced that this is sufficient to avoid breaking PMF with some existing drivers. diff --git a/src/drivers/driver.h b/src/drivers/driver.h index 2020184c5f94..e418eb20c6e6 100644 --- a/src/drivers/driver.h +++ b/src/drivers/driver.h @@ -1847,6 +1847,7 @@ struct wpa_driver_capa { #define WPA_DRIVER_CAPA_ENC_BIP_GMAC_256 0x00000400 #define WPA_DRIVER_CAPA_ENC_BIP_CMAC_256 0x00000800 #define WPA_DRIVER_CAPA_ENC_GTK_NOT_USED 0x00001000 +#define WPA_DRIVER_CAPA_ENC_BIP_KNOWN 0x00002000 /** Bitfield of supported cipher suites */ unsigned int enc; diff --git a/src/drivers/driver_nl80211_capa.c b/src/drivers/driver_nl80211_capa.c index 83868b78e6f0..fcd11c76e2e5 100644 --- a/src/drivers/driver_nl80211_capa.c +++ b/src/drivers/driver_nl80211_capa.c @@ -438,6 +438,8 @@ static void wiphy_info_cipher_suites(struct wiphy_info_data *info, if (tb == NULL) return; + info->capa->enc |= WPA_DRIVER_CAPA_ENC_BIP_KNOWN; + num = nla_len(tb) / sizeof(u32); ciphers = nla_data(tb); for (i = 0; i < num; i++) { diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c index 9a781e08c3a0..48d17396c274 100644 --- a/wpa_supplicant/wpa_supplicant.c +++ b/wpa_supplicant/wpa_supplicant.c @@ -7782,13 +7782,17 @@ int wpas_network_disabled(struct wpa_supplicant *wpa_s, struct wpa_ssid *ssid) int wpas_get_ssid_pmf(struct wpa_supplicant *wpa_s, struct wpa_ssid *ssid) { int pmf; + bool use_default = false; - if (ssid && ssid->ieee80211w != MGMT_FRAME_PROTECTION_DEFAULT) + if (ssid && ssid->ieee80211w != MGMT_FRAME_PROTECTION_DEFAULT) { pmf = ssid->ieee80211w; - else + } else { pmf = wpa_s->conf->pmf; + use_default = true; + } if (pmf == MGMT_FRAME_PROTECTION_OPTIONAL && + (use_default || (wpa_s->drv_enc & WPA_DRIVER_CAPA_ENC_BIP_KNOWN)) && !(wpa_s->drv_enc & WPA_DRIVER_CAPA_ENC_BIP)) { /* * Driver does not support BIP -- ignore pmf=1 default
On Wed, Aug 25, 2021 at 8:49 AM Jouni Malinen <j@w1.fi> wrote: > > What's the use case for this change? For my use case setting pmf=1 globally and leaving ieee80211w unset on the explicit network configurations does allow this code to connect to an optional network without PMF. I simply expected the explicit ieee80211w=1 would behave the same as the global pmf=1 setting in my case but instead it fails "to configure IGTK to the driver". > I'm not completely sure about the nl80211 cases since the BIP cipher > suite support indication might have been added later than the initial > PMF implementation. This may have resulted in there being no strict > rejection of BIP configuration with drivers that do not have explicit > indication for it in the supported ciphers list. Thank you for the insight. I had not thought of a driver supporting PMF without indicating support for BIP. > As such, it may be a > bit difficult to do this type of a change in wpa_supplicant without the > kernel interface(s) changing first to explicitly indicate whether PMF is > supported. This is likely out of the scope of my current needs. Additionally, that would require adding the explicit interface to non-nl80211 drivers as well wouldn't it? Thank you for clarifying the reasons behind these differences. Jeff
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c index 0d9b9caa5..75778a75e 100644 --- a/wpa_supplicant/wpa_supplicant.c +++ b/wpa_supplicant/wpa_supplicant.c @@ -7718,34 +7718,36 @@ int wpas_network_disabled(struct wpa_supplicant *wpa_s, struct wpa_ssid *ssid) int wpas_get_ssid_pmf(struct wpa_supplicant *wpa_s, struct wpa_ssid *ssid) { - if (ssid == NULL || ssid->ieee80211w == MGMT_FRAME_PROTECTION_DEFAULT) { - if (wpa_s->conf->pmf == MGMT_FRAME_PROTECTION_OPTIONAL && - !(wpa_s->drv_enc & WPA_DRIVER_CAPA_ENC_BIP)) { - /* - * Driver does not support BIP -- ignore pmf=1 default - * since the connection with PMF would fail and the - * configuration does not require PMF to be enabled. - */ - return NO_MGMT_FRAME_PROTECTION; - } + int pmf; - if (ssid && - (ssid->key_mgmt & - ~(WPA_KEY_MGMT_NONE | WPA_KEY_MGMT_WPS | - WPA_KEY_MGMT_IEEE8021X_NO_WPA)) == 0) { - /* - * Do not use the default PMF value for non-RSN networks - * since PMF is available only with RSN and pmf=2 - * configuration would otherwise prevent connections to - * all open networks. - */ - return NO_MGMT_FRAME_PROTECTION; - } + if (ssid && ssid->ieee80211w != MGMT_FRAME_PROTECTION_DEFAULT) + pmf = ssid->ieee80211w; + else + pmf = wpa_s->conf->pmf; - return wpa_s->conf->pmf; + if (pmf == MGMT_FRAME_PROTECTION_OPTIONAL && + !(wpa_s->drv_enc & WPA_DRIVER_CAPA_ENC_BIP)) { + /* + * Driver does not support BIP -- ignore pmf=1 default + * since the connection with PMF would fail and the + * configuration does not require PMF to be enabled. + */ + return NO_MGMT_FRAME_PROTECTION; } - return ssid->ieee80211w; + if (ssid && ssid->ieee80211w == MGMT_FRAME_PROTECTION_DEFAULT && + (ssid->key_mgmt & + ~(WPA_KEY_MGMT_NONE | WPA_KEY_MGMT_WPS | + WPA_KEY_MGMT_IEEE8021X_NO_WPA)) == 0) { + /* + * Do not use the default PMF value for non-RSN networks + * since PMF is available only with RSN and pmf=2 + * configuration would otherwise prevent connections to + * all open networks. + */ + return NO_MGMT_FRAME_PROTECTION; + } + return pmf; }
Networks configured with ieee80211w=1 will fail to connect to a PMF enabled AP during negotiation if the driver does not support PMF. Extend the existing global driver PMF capability check to apply when the network specific ieee80211w configuration value is set to optional. This allows networks configured with PMF as optional to make use of this existing driver check. Signed-off-by: Jeffery Miller <jefferymiller@google.com> --- wpa_supplicant/wpa_supplicant.c | 50 +++++++++++++++++---------------- 1 file changed, 26 insertions(+), 24 deletions(-)