On Tue, Oct 25, 2016 at 10:10:30AM -0700, Derrick Pallas wrote: > 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. Interesting.. I can understand the issue with positive case, but I don't see how this could have worked in the negative either before either of these commit or after both the commit and this fix.. Would you be able to share hostapd debug logs showing the tests failing without this new proposed commit and then passing with this commit applied? The main issue with this new patch is that it allows the STA to remain associated while even if the RADIUS server were to reject the connection afterwards. hostapd_acl_recv_radius() in src/ap/ieee802_11_auth.c would need some additional code to handle the RADIUS_CODE_ACCESS_REJECT case by disconnecting the relevant STA at the time Access-Reject is received. I don't see such code there.. I don't think this RADIUS-based MAC ACL was ever designed or supposed to work with WLAN drivers that implement AP SME, i.e., this has worked fine only for the case where hostapd processes the Authentication frames. The commit to add a check for HOSTAPD_ACL_ACCEPT in hostapd_notif_assoc() makes this explicit. I think I would be fine with extending this for AP SME in driver cases as long as it does not introduce potential security issues. This would likely need two additional changes to this proposed patch: (1) hostapd_acl_recv_radius() to disconnect the STA on receiving Access-Reject (if the STA is associated at that point in time) (2) timeout to disconnect the STA if no RADIUS response is received (this should be able to use ap_handle_timer() instead of having to come up with something completely new; schedule that when hostapd_notif_assoc() gets HOSTAPD_ACL_PENDING and allows the STA to proceed) (and see below for couple more possibly needed items..) > 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? One inconvenient part here is in mac80211_hwsim test cases not supporting the AP SME in the driver case and because of this, there are no automated test cases for this in hostap.git. If someone is interested in working on those two items I listed above, it would be interested to get them tested and assuming that works properly for the two cases where the STA needs to get disconnected due to lack of Access-Accept, I'd like to get these changes included. I don't think I'd apply this one-liner, though, before those two other changes are available due to the potential security issues this could open accidentally (sure, those were there before v2.6, but now that the change in v2.6 blocks this, I don't really want to revert it without more complete fix). > On Tue, Oct 25, 2016 at 8:20 AM, M. Braun <mbrrc@xxxxxxxxxxxxx> wrote: > > 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. Hmm.. This may actually add item (3) to my list above, i.e., the Access-Request message does indeed need to be sent out. Maybe the hostapd_check_acl() call in hostapd_notif_assoc() would need to be changed to use hostapd_allowed_address().. The inconvenient part here is in having to accept the association before knowing the RADIUS server response. I guess we could complete this using the port authorization mechanism even in open network (which would be the most likely used for MAC ACL) so that the STA can remain associated, but it cannot really send any data frames (or receive unicast data frames). So that maybe be item (4) in the list.. -- Jouni Malinen PGP id EFC895FA _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap