diff mbox series

No OWE transition mode element on hidden OWE network

Message ID 47393b395140d0a6a3a583a4c2d2f503f2e2e872.camel@gmail.com
State Rejected
Headers show
Series No OWE transition mode element on hidden OWE network | expand

Commit Message

James Prestwood Sept. 9, 2021, 11:09 p.m. UTC
Hi,

While playing around with OWE transition mode I noticed the hidden OWE
network has no OWE transition mode element. The two network configs are
attached.

According to the OWE Spec v1.1, section 2.2.1:

"The OWE BSS shall include the OWE Transition Mode element in all
Beacon and Probe Response frames to encapsulate the BSSID and SSID of
the Open BSS."

I figured it was a misconfiguration but then I found the following code
in src/ap/ieee802_11_shared.c:

static int hostapd_eid_owe_trans_enabled(struct hostapd_data *hapd)
{
	return hapd->conf->owe_transition_ssid_len > 0 &&
		!is_zero_ether_addr(hapd->conf->owe_transition_bssid);
}

This is called prior to appending the OWE transition element so for the
hidden SSID (where ssid_len < 0) it returns false and the IE is never
built/appended.

Removing the SSID length check seems to fix this and I see the OWE
transition element for the hidden OWE network. Attached is the patch to
remove this length check.

Thanks,
James
ssid=owe-hidden
bssid=a6:44:ce:d8:61:6f
channel=1
ignore_broadcast_ssid=1
ieee80211w=1

wpa=2
wpa_key_mgmt=OWE
rsn_pairwise=CCMP
owe_transition_ssid="transition"
owe_transition_bssid=fe:e1:de:ce:a5:ed
channel=1
ssid=transition
bssid=fe:e1:de:ce:a5:ed
owe_transition_ssid="owe-hidden"
owe_transition_bssid=a6:44:ce:d8:61:6f

Comments

James Prestwood Sept. 15, 2021, 11:28 p.m. UTC | #1
Ping, anyone seen this?

From my end the path forward without this patch is to use
vendor_elements. This also adds the benefit of being able to include
channel/band information which isn't supported by these options.

My fear is that these options would be deployed as-is and cause
interoperability problems with IWD. And IWD is not planning on
supporting networks where the IE is missing, the book keeping and
lookup logic becomes much more complex.

I realize this code was written in 2017, long before any spec was even
released and OWE transitional networks are likely not deployed anywhere
yet... But in any case it would be great to get this taken care of now
and not end up in the same situations we have had with SAE in actual
consumer products:

http://lists.infradead.org/pipermail/hostap/2021-September/039842.html

On Thu, 2021-09-09 at 16:09 -0700, James Prestwood wrote:
> Hi,
> 
> While playing around with OWE transition mode I noticed the hidden OWE
> network has no OWE transition mode element. The two network configs are
> attached.
> 
> According to the OWE Spec v1.1, section 2.2.1:
> 
> "The OWE BSS shall include the OWE Transition Mode element in all
> Beacon and Probe Response frames to encapsulate the BSSID and SSID of
> the Open BSS."
> 
> I figured it was a misconfiguration but then I found the following code
> in src/ap/ieee802_11_shared.c:
> 
> static int hostapd_eid_owe_trans_enabled(struct hostapd_data *hapd)
> {
>         return hapd->conf->owe_transition_ssid_len > 0 &&
>                 !is_zero_ether_addr(hapd->conf->owe_transition_bssid);
> }
> 
> This is called prior to appending the OWE transition element so for the
> hidden SSID (where ssid_len < 0) it returns false and the IE is never
> built/appended.
> 
> Removing the SSID length check seems to fix this and I see the OWE
> transition element for the hidden OWE network. Attached is the patch to
> remove this length check.
> 
> Thanks,
> James
Jouni Malinen Oct. 19, 2021, 1:06 p.m. UTC | #2
On Thu, Sep 09, 2021 at 04:09:02PM -0700, James Prestwood wrote:
> While playing around with OWE transition mode I noticed the hidden OWE
> network has no OWE transition mode element. The two network configs are
> attached.

That does not match what I see in my tests, i.e., I do see the OWE
Transition Mode element being added to Beacon frames from both BSSs.

> I figured it was a misconfiguration but then I found the following code
> in src/ap/ieee802_11_shared.c:
> 
> static int hostapd_eid_owe_trans_enabled(struct hostapd_data *hapd)
> {
> 	return hapd->conf->owe_transition_ssid_len > 0 &&
> 		!is_zero_ether_addr(hapd->conf->owe_transition_bssid);
> }
> 
> This is called prior to appending the OWE transition element so for the
> hidden SSID (where ssid_len < 0) it returns false and the IE is never
> built/appended.

I'm not sure what you mean with hidden SSID and ssid_len < 0 (I guess
you meant == 0 here).. That is a comparison on the
owe_transition_ssid_len, i.e., the length of the owe_transition_ssid
parameter in the BSS configuration. If there is OWE transition in place,
that entry needs to be set and needs to have the correct SSID of the
other BSS. In other words, owe_transition_ssid_len cannot be 0 in valid
configuration.

> Removing the SSID length check seems to fix this and I see the OWE
> transition element for the hidden OWE network. Attached is the patch to
> remove this length check.

That would be allowing an invalid configuration to be used. Valid
owe_transition_ssid needs to be present in the configuration.

And it actually is in the example configuration you attached:

> ssid=owe-hidden
> bssid=a6:44:ce:d8:61:6f
> channel=1
> ignore_broadcast_ssid=1
> ieee80211w=1
> 
> wpa=2
> wpa_key_mgmt=OWE
> rsn_pairwise=CCMP
> owe_transition_ssid="transition"
> owe_transition_bssid=fe:e1:de:ce:a5:ed

There is a non-empty owe_transition_ssid value here, so I don't see why
you would need to modify the check in hostapd_eid_owe_trans_enabled().

> channel=1
> ssid=transition
> bssid=fe:e1:de:ce:a5:ed
> owe_transition_ssid="owe-hidden"
> owe_transition_bssid=a6:44:ce:d8:61:6f

Similarly here, owe_transition_ssid is set.
James Prestwood Oct. 19, 2021, 6:49 p.m. UTC | #3
Hi Jouni,

On Tue, 2021-10-19 at 16:06 +0300, Jouni Malinen wrote:
> On Thu, Sep 09, 2021 at 04:09:02PM -0700, James Prestwood wrote:
> > While playing around with OWE transition mode I noticed the hidden
> > OWE
> > network has no OWE transition mode element. The two network configs
> > are
> > attached.
> 
> That does not match what I see in my tests, i.e., I do see the OWE
> Transition Mode element being added to Beacon frames from both BSSs.
> 
> > I figured it was a misconfiguration but then I found the following
> > code
> > in src/ap/ieee802_11_shared.c:
> > 
> > static int hostapd_eid_owe_trans_enabled(struct hostapd_data *hapd)
> > {
> >         return hapd->conf->owe_transition_ssid_len > 0 &&
> >                 !is_zero_ether_addr(hapd->conf-
> > >owe_transition_bssid);
> > }
> > 
> > This is called prior to appending the OWE transition element so for
> > the
> > hidden SSID (where ssid_len < 0) it returns false and the IE is
> > never
> > built/appended.
> 
> I'm not sure what you mean with hidden SSID and ssid_len < 0 (I guess
> you meant == 0 here).. That is a comparison on the
> owe_transition_ssid_len, i.e., the length of the owe_transition_ssid
> parameter in the BSS configuration. If there is OWE transition in
> place,
> that entry needs to be set and needs to have the correct SSID of the
> other BSS. In other words, owe_transition_ssid_len cannot be 0 in
> valid
> configuration.
> 
> > Removing the SSID length check seems to fix this and I see the OWE
> > transition element for the hidden OWE network. Attached is the
> > patch to
> > remove this length check.
> 
> That would be allowing an invalid configuration to be used. Valid
> owe_transition_ssid needs to be present in the configuration.
> 
> And it actually is in the example configuration you attached:
> 
> > ssid=owe-hidden
> > bssid=a6:44:ce:d8:61:6f
> > channel=1
> > ignore_broadcast_ssid=1
> > ieee80211w=1
> > 
> > wpa=2
> > wpa_key_mgmt=OWE
> > rsn_pairwise=CCMP
> > owe_transition_ssid="transition"
> > owe_transition_bssid=fe:e1:de:ce:a5:ed
> 
> There is a non-empty owe_transition_ssid value here, so I don't see
> why
> you would need to modify the check in
> hostapd_eid_owe_trans_enabled().
> 
> > channel=1
> > ssid=transition
> > bssid=fe:e1:de:ce:a5:ed
> > owe_transition_ssid="owe-hidden"
> > owe_transition_bssid=a6:44:ce:d8:61:6f
> 
> Similarly here, owe_transition_ssid is set.

Thanks for the reply. I'm not sure exactly what behavior I was seeing
before because I retested this and indeed it works as I would expect
and as you explained. We went as far as writing all our tests with
vendor_elements because "it didn't work". Anyways, thank you and sorry
for the noise.

- James
diff mbox series

Patch

From 49bc686b2d05acef909449e61f1492e346a7dff4 Mon Sep 17 00:00:00 2001
From: James Prestwood <prestwoj@gmail.com>
Date: Thu, 9 Sep 2021 16:05:24 -0700
Subject: [PATCH] owe: remove ssid length check for OWE transition element

This removes the SSID length check when appending the OWE transition
element. With this check in place the transition element is never
appended to the hidden OWE network.

Signed-off-by: James Prestwood <prestwoj@gmail.com>
---
 src/ap/ieee802_11_shared.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/ap/ieee802_11_shared.c b/src/ap/ieee802_11_shared.c
index 4bff9e591..fc3b2e01e 100644
--- a/src/ap/ieee802_11_shared.c
+++ b/src/ap/ieee802_11_shared.c
@@ -812,8 +812,7 @@  u8 hostapd_mbo_ie_len(struct hostapd_data *hapd)
 #ifdef CONFIG_OWE
 static int hostapd_eid_owe_trans_enabled(struct hostapd_data *hapd)
 {
-	return hapd->conf->owe_transition_ssid_len > 0 &&
-		!is_zero_ether_addr(hapd->conf->owe_transition_bssid);
+	return !is_zero_ether_addr(hapd->conf->owe_transition_bssid);
 }
 #endif /* CONFIG_OWE */
 
-- 
2.31.1