diff mbox series

[v2,01/11] nl80211: also check for no preauth feature

Message ID 20200411102527.154154-2-markus.theil@tu-ilmenau.de
State Superseded
Headers show
Series nl80211: rx path for control port frames (enabled only for wpa_supplicant) | expand

Commit Message

Markus Theil April 11, 2020, 10:25 a.m. UTC
Before Linux 5.7 all pre-auth frames are forwared over the nl80211 ctrl
port, if it is registered. hostap and wpa_supplicant currently do not assume
this behavior, as pre-auth frames should be handled as ordinary data frames
in the kernel. Checking against the NL80211_EXT_FEATURE_CONTROL_PORT_NO_PREAUTH
feature flag allows us to disable this behavior later on ctrl port registration.

No new capa->flags value is introduced, instead WPA_DRIVER_FLAGS_CONTROL_PORT is
only set now, if this feature is present. Without this feature, only control
port tx is possible without breaking pre-auth features.

Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de>
---
 src/drivers/driver_nl80211_capa.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Jouni Malinen April 18, 2020, 1:57 p.m. UTC | #1
On Sat, Apr 11, 2020 at 12:25:17PM +0200, Markus Theil wrote:
> Before Linux 5.7 all pre-auth frames are forwared over the nl80211 ctrl
> port, if it is registered. hostap and wpa_supplicant currently do not assume
> this behavior, as pre-auth frames should be handled as ordinary data frames
> in the kernel. Checking against the NL80211_EXT_FEATURE_CONTROL_PORT_NO_PREAUTH
> feature flag allows us to disable this behavior later on ctrl port registration.
> 
> No new capa->flags value is introduced, instead WPA_DRIVER_FLAGS_CONTROL_PORT is
> only set now, if this feature is present. Without this feature, only control
> port tx is possible without breaking pre-auth features.

Wouldn't this break the currently working EAPOL TX over control port
with older kernel versions?

> diff --git a/src/drivers/driver_nl80211_capa.c b/src/drivers/driver_nl80211_capa.c
> @@ -439,7 +439,9 @@ static void wiphy_info_ext_feature_flags(struct wiphy_info_data *info,
>  	if (ext_feature_isset(ext_features, len,
> -			      NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211))
> +			      NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211) &&
> +	    ext_feature_isset(ext_features, len,
> +			      NL80211_EXT_FEATURE_CONTROL_PORT_NO_PREAUTH))
>  		capa->flags |= WPA_DRIVER_FLAGS_CONTROL_PORT;

I was expecting the older capability
NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211 to enable TX-only and then
add RX on top of that if NL80211_EXT_FEATURE_CONTROL_PORT_NO_PREAUTH is
present. Is there a reason for not doing that? There have been two years
of kernel releases with NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211
while this new NL80211_EXT_FEATURE_CONTROL_PORT_NO_PREAUTH was added
just now, so it would seem that this patch would push out deployment of
this capability for couple of years..
Markus Theil April 18, 2020, 7:19 p.m. UTC | #2
On 4/18/20 3:57 PM, Jouni Malinen wrote:
> On Sat, Apr 11, 2020 at 12:25:17PM +0200, Markus Theil wrote:
>> Before Linux 5.7 all pre-auth frames are forwared over the nl80211 ctrl
>> port, if it is registered. hostap and wpa_supplicant currently do not assume
>> this behavior, as pre-auth frames should be handled as ordinary data frames
>> in the kernel. Checking against the NL80211_EXT_FEATURE_CONTROL_PORT_NO_PREAUTH
>> feature flag allows us to disable this behavior later on ctrl port registration.
>>
>> No new capa->flags value is introduced, instead WPA_DRIVER_FLAGS_CONTROL_PORT is
>> only set now, if this feature is present. Without this feature, only control
>> port tx is possible without breaking pre-auth features.
> Wouldn't this break the currently working EAPOL TX over control port
> with older kernel versions?
Yes.
>> diff --git a/src/drivers/driver_nl80211_capa.c b/src/drivers/driver_nl80211_capa.c
>> @@ -439,7 +439,9 @@ static void wiphy_info_ext_feature_flags(struct wiphy_info_data *info,
>>  	if (ext_feature_isset(ext_features, len,
>> -			      NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211))
>> +			      NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211) &&
>> +	    ext_feature_isset(ext_features, len,
>> +			      NL80211_EXT_FEATURE_CONTROL_PORT_NO_PREAUTH))
>>  		capa->flags |= WPA_DRIVER_FLAGS_CONTROL_PORT;
> I was expecting the older capability
> NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211 to enable TX-only and then
> add RX on top of that if NL80211_EXT_FEATURE_CONTROL_PORT_NO_PREAUTH is
> present. Is there a reason for not doing that? There have been two years
> of kernel releases with NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211
> while this new NL80211_EXT_FEATURE_CONTROL_PORT_NO_PREAUTH was added
> just now, so it would seem that this patch would push out deployment of
> this capability for couple of years..
>  

Good point, I'll change it to behave that way.

Should the u64 flags in src/drivers/driver.h, which holds WPA driver, flags then become an array with some additional macros for setting and querying flags, when the 65th flag for control_port_rx is added?
Jouni Malinen April 19, 2020, 9:35 a.m. UTC | #3
On Sat, Apr 18, 2020 at 09:19:19PM +0200, Markus Theil wrote:
> Should the u64 flags in src/drivers/driver.h, which holds WPA driver, flags then become an array with some additional macros for setting and querying flags, when the 65th flag for control_port_rx is added?

I added another u64 flags2 for additional capability bits with
WPA_DRIVER_FLAGS2_CONTROL_PORT_RX being the first user of that.
diff mbox series

Patch

diff --git a/src/drivers/driver_nl80211_capa.c b/src/drivers/driver_nl80211_capa.c
index b4fed9ea8..cd0ecf75d 100644
--- a/src/drivers/driver_nl80211_capa.c
+++ b/src/drivers/driver_nl80211_capa.c
@@ -439,7 +439,9 @@  static void wiphy_info_ext_feature_flags(struct wiphy_info_data *info,
 		capa->flags |= WPA_DRIVER_FLAGS_FTM_RESPONDER;
 
 	if (ext_feature_isset(ext_features, len,
-			      NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211))
+			      NL80211_EXT_FEATURE_CONTROL_PORT_OVER_NL80211) &&
+	    ext_feature_isset(ext_features, len,
+			      NL80211_EXT_FEATURE_CONTROL_PORT_NO_PREAUTH))
 		capa->flags |= WPA_DRIVER_FLAGS_CONTROL_PORT;
 
 	if (ext_feature_isset(ext_features, len,