| Message ID | 20181205102401.17810-9-arnout@mind.be |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v3,01/12] hostapd: Add Multi-AP protocol support | expand |
On 05/12/2018 11:23, Arnout Vandecappelle (Essensium/Mind) wrote: > + if (hapd->conf->multi_ap & FRONTHAUL_BSS) { > + if (multi_ap_value == MULTI_AP_BACKHAUL_STA) { > + hostapd_logger(hapd, sta->addr, > + HOSTAPD_MODULE_IEEE80211, > + HOSTAPD_LEVEL_INFO, > + "Backhaul STA tries to associate with fronthaul-only BSS"); > + return WLAN_STATUS_ASSOC_DENIED_UNSPEC; > + } Turns out that this bit was a mistake. It breaks WPS. In Multi-AP, WPS is done on the fronthaul BSS. The backhaul STA will associate with the fronthaul BSS and provide the Multi-AP IE. This will cause the fronthaul BSS to provide credentials for the backhaul BSS instead of for itself. So, if the fronthaul BSS is fronthaul-only, it may indeed get an association request with a backhaul STA Multi-AP IE... Since the specification doesn't say explicitly that a bSTA trying to associate on a fronthaul-only BSS should be rejected, I think the easiest is to remove this check again. I'll do that as part of my refresh of patches 10-12. Regards, Arnout
> Since the specification doesn't say explicitly that a bSTA trying to associate > on a fronthaul-only BSS should be rejected, I think the easiest is to remove > this check again. Missed it as well. I think a bSTA trying to associate on a "fronthaul-only" BSS is a nominal case, so this check should indeed be removed as there is no reason to reject the bSTA. However, we may have consider the case where the device has only fronthaul BSSes, no backhaul BSS at all. I can't find a way in the Multi-AP specifications for a MAP Controller to provide a MAP Agent with the backhaul credentials, while not actually configuring a backhaul BSS at all. If that's correct, in theory, we could have a situation where there are one or more frounthaul BSSes configured (i.e., multi_ap=2) but no "multi_ap_backhaul_*" properties configured (cause the device has no mean to know them). When one of these BSSes with multi_ap=2 receives a WPS M1 from a "bSTA", it does not know the backhaul BSS credentials, so what credentials does it provide? Providing the credentials of the fronthaul BSS would be wrong per the MAP specs, so maybe rejecting it with an error would be best. What do you think? -marianna
On 07/01/2019 15:30, Marianna Carrera wrote: >> Since the specification doesn't say explicitly that a bSTA trying to associate >> on a fronthaul-only BSS should be rejected, I think the easiest is to remove >> this check again. > > Missed it as well. I think a bSTA trying to associate on a "fronthaul-only" BSS is a nominal case, so this check should indeed be removed as there is no reason to reject the bSTA. > > However, we may have consider the case where the device has only fronthaul BSSes, no backhaul BSS at all. > I can't find a way in the Multi-AP specifications for a MAP Controller to provide a MAP Agent with the backhaul credentials, while not actually configuring a backhaul BSS at all. > > If that's correct, in theory, we could have a situation where there are one or more frounthaul BSSes configured (i.e., multi_ap=2) but no "multi_ap_backhaul_*" properties configured (cause the device has no mean to know them). When one of these BSSes with multi_ap=2 receives a WPS M1 from a "bSTA", it does not know the backhaul BSS credentials, so what credentials does it provide? > Providing the credentials of the fronthaul BSS would be wrong per the MAP specs, so maybe rejecting it with an error would be best. Good point. Patch 11/12 adds: + if (wps->peer_dev.multi_ap_ext == MULTI_AP_BACKHAUL_STA && + wps->wps->multi_ap_backhaul_ssid_len) so that should become something like: if (wps->peer_dev.multi_ap_ext == MULTI_AP_BACKHAUL_STA) { if (!wps->wps->multi_ap_backhaul_ssid_len) { return ERROR; } ... } That would reject all bSTA requests if no backhaul BSS is configured, even if hostapd is otherwise multiap-unaware. Not sure if that is approprate... On the other hand, if a bSTA does WPS to an AP which is not multi-AP, it shouldn't use those credentials anyway, so it's actually good to reject right away. Regards, Arnout > > What do you think? > > -marianna >
diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c index fec5882e0..63ab27cb3 100644 --- a/src/ap/ieee802_11.c +++ b/src/ap/ieee802_11.c @@ -2224,31 +2224,53 @@ static u16 check_wmm(struct hostapd_data *hapd, struct sta_info *sta, return WLAN_STATUS_SUCCESS; } -static u16 hostapd_validate_multi_ap_ie(struct hostapd_data *hapd, - struct sta_info *sta, - struct ieee802_11_elems *elems) +static u16 check_multi_ap(struct hostapd_data *hapd, struct sta_info *sta, + const u8 *multi_ap_ie, size_t multi_ap_len) { - const u8 *pos, *map_sub_elem; - size_t len; + u8 multi_ap_value = 0; - if (!hapd->conf->multi_ap || !elems->multi_ap) + sta->flags &= ~WLAN_STA_MULTI_AP; + + if (!hapd->conf->multi_ap) return WLAN_STATUS_SUCCESS; - pos = elems->multi_ap + 4;/* OUI[3] and OUT_TYPE 1 */ - len = elems->multi_ap_len - 4; + if (multi_ap_ie) { + const u8 *multi_ap_subelem = get_ie(multi_ap_ie + 4, + multi_ap_len - 4, + MULTI_AP_SUB_ELEM_TYPE); + if (multi_ap_subelem && + multi_ap_subelem[1] == 1) + multi_ap_value = multi_ap_subelem[2]; + else { + hostapd_logger(hapd, sta->addr, + HOSTAPD_MODULE_IEEE80211, + HOSTAPD_LEVEL_INFO, + "Multi-AP IE has missing or invalid Multi-AP subelement"); + return WLAN_STATUS_INVALID_IE; + } + } - sta->flags &= ~WLAN_STA_MULTI_AP; + if (multi_ap_value == MULTI_AP_BACKHAUL_STA) + sta->flags |= WLAN_STA_MULTI_AP; - map_sub_elem = get_ie(pos, len, MULTI_AP_SUB_ELEM_TYPE); + if (hapd->conf->multi_ap & BACKHAUL_BSS && + multi_ap_value == MULTI_AP_BACKHAUL_STA) + return WLAN_STATUS_SUCCESS; - if (map_sub_elem) { - if (map_sub_elem[1] < 1) - return WLAN_STATUS_UNSPECIFIED_FAILURE; - if (map_sub_elem[2] & MULTI_AP_BACKHAUL_STA) - sta->flags |= WLAN_STA_MULTI_AP; + if (hapd->conf->multi_ap & FRONTHAUL_BSS) { + if (multi_ap_value == MULTI_AP_BACKHAUL_STA) { + hostapd_logger(hapd, sta->addr, + HOSTAPD_MODULE_IEEE80211, + HOSTAPD_LEVEL_INFO, + "Backhaul STA tries to associate with fronthaul-only BSS"); + return WLAN_STATUS_ASSOC_DENIED_UNSPEC; + } + return WLAN_STATUS_SUCCESS; } - - return WLAN_STATUS_SUCCESS; + hostapd_logger(hapd, sta->addr, HOSTAPD_MODULE_IEEE80211, + HOSTAPD_LEVEL_INFO, + "Non-Multi-AP STA tries to associate with backhaul-only BSS"); + return WLAN_STATUS_ASSOC_DENIED_UNSPEC; } static u16 copy_supp_rates(struct hostapd_data *hapd, struct sta_info *sta, @@ -2507,7 +2529,7 @@ static u16 check_assoc_ies(struct hostapd_data *hapd, struct sta_info *sta, if (resp != WLAN_STATUS_SUCCESS) return resp; - resp = hostapd_validate_multi_ap_ie(hapd, sta, &elems); + resp = check_multi_ap(hapd, sta, elems.multi_ap, elems.multi_ap_len); if (resp != WLAN_STATUS_SUCCESS) return resp;
The validation of the multi-AP IE in hostapd is not very complete. * When the IE is incorrectly constructed, return WLAN_STATUS_INVALID_IE. This might not be entirely the right thing to do, since that status is in principle reserved for IEs specified in IEEE 802.11-2016, but it makes sense to use that for incorrect subelements as well, and it's done in other places. * When the AP is configured as fronthaul-only, association requests for backhaul STA are rejected. * When the AP is configured as backhaul-only, association requests without backhaul STA are rejected. * A multi-AP IE with a value different than MULTI_AP_BACKHAUL_STA is ignored (i.e. treated the same as if it was missing). * Rejections use reason WLAN_STATUS_ASSOC_DENIED_UNSPEC instead of WLAN_STATUS_UNSPECIFIED_FAILURE - that seems a better match with the IEEE 802.11-2016 specification. * Set the WLAN_STA_MULTI_AP based on the IE, independent of whether it is a backhaul or fronthaul AP. This makes sure that the Association Response contains the IE even if it is a rejection. None of the above is explicitly specified in the Multi-AP Specification. However, it is pretty clear that we only want backhaul stations on a backhaul-only AP and only fronthaul stations on a fronhaul-only AP, otherwise the distinction is pointless. Finally, rename hostapd_validate_multi_ap_ie() to check_multi_ap() and directly give it the multi_ap_ie as argument rather than the ieee802_11_elems, since that is how most other functions in that file work. Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> Cc: Venkateswara Naralasetty <vnaralas@codeaurora.org> --- src/ap/ieee802_11.c | 58 +++++++++++++++++++++++++++++++-------------- 1 file changed, 40 insertions(+), 18 deletions(-)