Howdy,
During our bump from hostap 2.5 to 2.6, our basic feature test flagged
this as a regression, specifically MAC-based auth using an external
RADIUS server. After a bisect, the referenced commit was identified as
the source. The code in ap/drv_callbacks was newly added, i.e. no
check was done previously, and hostapd_check_acl is definitely
returning pending here. This commit fixes the issue in our automated
test, for both the positive case and the negative case.
Unfortunately, I am not very familiar with hostap's internals, so I'm
not sure I can comment on the reasoning you lay out. Perhaps there is
a different bug that this change masks. Is there any testing or
debugging you'd like me to run on my side?
~Derrick
On Tue, Oct 25, 2016 at 8:20 AM, M. Braun <mbrrc@xxxxxxxxxxxxx> wrote:
> Hi,
>
> hostapd_check_acl can only return PENDING if macaddr_acl ==
> USE_EXTERNAL_RADIUS_AUTH.
>
> In that case, either
> a) hostapd_allowed_address is used before and so the RADIUS reply came in
> before
> association is completed and hostapd_notif_assoc is called or
> b) hostapd_allowed_address is not called (e.g. SME in driver) and so
> macaddr_acl == USE_EXTERNAL_RADIUS_AUTH is not implemented.
>
> In case b) this change would accept a station bypassing RADIUS, while
> currently
> admin would need to choose a different macaddr_acl value to disable RADIUS
> withmacaddr_acl
> when using SME in driver.
>
> Right?
>
> Regards,
> M. Braun
>
>
>
> Am 25.10.2016 02:05, schrieb Derrick Pallas:
>>
>> The commit
>>
>> hostapd: Process MAC ACLs on a station association event (SME in
>> driver)
>>
>> added a MAC ACL check to hostapd_notif_assoc. This check disconnects the
>> client if the response is not ACCEPT, but the function can actually return
>> PENDING too, as in the case of 802.1x MAC-based auth. It feels like the
>> author probably meant to disconnect the client if the response is REJECT,
>> but not ACCEPT or PENDING instead.
>>
>> Signed-off-by: Derrick Pallas <pallas@xxxxxxxxxx>
>> ---
>> src/ap/drv_callbacks.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/ap/drv_callbacks.c b/src/ap/drv_callbacks.c
>> index 3552b3e..f065995 100644
>> --- a/src/ap/drv_callbacks.c
>> +++ b/src/ap/drv_callbacks.c
>> @@ -124,7 +124,7 @@ int hostapd_notif_assoc(struct hostapd_data *hapd,
>> const u8 *addr,
>> * conflicting ACL rules.
>> */
>> if (hapd->iface->drv_max_acl_mac_addrs == 0 &&
>> - hostapd_check_acl(hapd, addr, NULL) != HOSTAPD_ACL_ACCEPT) {
>> + hostapd_check_acl(hapd, addr, NULL) == HOSTAPD_ACL_REJECT) {
>> wpa_printf(MSG_INFO, "STA " MACSTR " not allowed to
>> connect",
>> MAC2STR(addr));
>> reason = WLAN_REASON_UNSPECIFIED;
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
