Hi, > For your second comment. I'm not sure. The filter logic favors an > entry in the accept ACL over an entry in the deny ACL. it depends on the place you're looking at. You're right for hostapd_check_acl, but not for hostapd_set_acl. Configuration option macaddr_acl values seems to indicate that only either - accept or deny list - should be used from manual configuration. So I think using the new API will make making mistakes easy as commanding DENY_ACL will not result in the station being denied depending on macaddr_acl value and accept_acl rules. Additionally, I don't see hostapd_drv_set_acl being called from your new functions, so the driver might still have the old acl configured. As hostapd_set_acl does not offload for macaddr_acl = 2 (RADIUS), this does not affect current code. Regards, M. Braun Am 03.10.2016 um 17:37 schrieb Kevin Mahoney: > Hi M. Braun, > > There also doesn't seem to be any error checking for a case where a MAC entry exists in both the accept and deny ACLs. I assumed it would be left up to the control logic to decide how to manage the lists based on this logic. Please let me know if this assumption is not valid. > > Best regards, > > Kevin > > > From: michael-dev@xxxxxxxxxxxxx [mailto:michael-dev@xxxxxxxxxxxxx] > Sent: Thursday, September 29, 2016 2:33 AM > To: Kevin Mahoney > Cc: hostap@xxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH] Added hostapd/hostapd_cli support for adding/removing a STA from the deny/accept ACL > > Hi, > > I don't think deny_acl needs TODO vlan because VLAN does not matter with deny rules. > Second, shouldn't adding to accept automatically remove from deny and vica versa? > > Regards, > M. Braun > Am 28. September 2016 23:01:12 MESZ, schrieb Kevin Mahoney <k.mahoney@xxxxxxxxxxxxx>: > This patch provides the ability to manage the deny and accept ACLs dynamically without having to reload the configuration. > > Signed-off-by: Kevin Mahoney <k.mahoney@xxxxxxxxxxxxx> > --- > hostapd/ctrl_iface.c | 6 +++ > hostapd/hostapd_cli.c | 32 ++++++++++++ > src/ap/ctrl_iface_ap.c | 136 +++++++++++++++++++++++++++++++++++++++++++++++++ > src/ap/ctrl_iface_ap.h | 4 ++ > 4 files changed, 178 insertions(+) > > diff --git a/hostapd/ctrl_iface.c b/hostapd/ctrl_iface.c > index d7db4a7..fbdc8a8 100644 > --- a/hostapd/ctrl_iface.c > +++ b/hostapd/ctrl_iface.c > @@ -2361,6 +2361,12 @@ static int hostapd_ctrl_iface_receive_process(struct hostapd_data *hapd, > } else if (os_strncmp(buf, "NEW_STA ", 8) == 0) { > if (hostapd_ctrl_iface_new_sta(hapd, buf + 8)) > reply_len = -1; > + } else if (os_strncmp(buf, "DENY_STA ", 9) == 0) { > + if (hostapd_ctrl_iface_deny_sta(hapd, buf + 9, reply, reply_size)) > + reply_len = -1; > + } else if (os_strncmp(buf, "ACCEPT_STA ", 11) == 0) { > + if (hostapd_ctrl_iface_accept_sta(hapd, buf + 11, reply, reply_size)) > + reply_len = -1; > } else if (os_strncmp(buf, "DEAUTHENTICATE ", 15) == 0) { > if (hostapd_ctrl_iface_deauthenticate(hapd, buf + 15)) > reply_len = -1; > diff --git a/hostapd/hostapd_cli.c b/hostapd/hostapd_cli.c > index 5e62542..103ee7a 100644 > --- a/hostapd/hostapd_cli.c > +++ b/hostapd/hostapd_cli.c > @@ -300,6 +300,34 @@ static int hostapd_cli_cmd_new_sta(struct wpa_ctrl *ctrl, int argc, > } > > > +static int hostapd_cli_cmd_deny_sta(struct wpa_ctrl *ctrl, int argc, > + char *argv[]) > +{ > + char buf[64]; > + if (argc != 1) { > + printf("Invalid 'deny_sta' command - exactly one argument, STA " > + "address, is required.\n"); > + return -1; > + } > + snprintf(buf, sizeof(buf), "DENY_STA %s", argv[0]); > + return wpa_ctrl_command(ctrl, buf); > +} > + > + > +static int hostapd_cli_cmd_accept_sta(struct wpa_ctrl *ctrl, int argc, > + char *argv[]) > +{ > + char buf[64]; > + if (argc != 1) { > + printf("Invalid 'accept_sta' command - exactly one argument, STA " > + "address, is required.\n"); > + return -1; > + } > + snprintf(buf, > sizeof(buf), "ACCEPT_STA %s", argv[0]); > + return wpa_ctrl_command(ctrl, buf); > +} > + > + > static int hostapd_cli_cmd_deauthenticate(struct wpa_ctrl *ctrl, int argc, > char *argv[]) > { > @@ -1281,6 +1309,10 @@ static const struct hostapd_cli_cmd hostapd_cli_commands[] = { > "= get MIB variables for all stations" }, > { "new_sta", hostapd_cli_cmd_new_sta, NULL, > "<addr> = add a new station" }, > + { "deny_sta", hostapd_cli_cmd_deny_sta, NULL, > + "<addr> = add a station to the deny ACL" }, > + { "accept_sta", hostapd_cli_cmd_accept_sta, NULL, > + > "<addr> = add a station to the accept ACL" }, > { "deauthenticate", hostapd_cli_cmd_deauthenticate, > hostapd_complete_deauthenticate, > "<addr> = deauthenticate a station" }, > diff --git a/src/ap/ctrl_iface_ap.c b/src/ap/ctrl_iface_ap.c > index 3680fda..58abb93 100644 > --- a/src/ap/ctrl_iface_ap.c > +++ b/src/ap/ctrl_iface_ap.c > @@ -251,6 +251,142 @@ int hostapd_ctrl_iface_sta_next(struct hostapd_data *hapd, const char *txtaddr, > } > > > +static int hostapd_acl_comp(const void *a, const void *b) > +{ > + const struct mac_acl_entry *aa = a; > + const struct mac_acl_entry *bb = b; > + return os_memcmp(aa->addr, bb->addr, sizeof(macaddr)); > +} > + > + > +int hostapd_ctrl_iface_deny_sta(struct hostapd_data *hapd, const char > *txtaddr, > + char *buf, size_t buflen) > +{ > + u8 addr[ETH_ALEN]; > + struct mac_acl_entry **acl = &hapd->conf->deny_mac; > + struct mac_acl_entry *newacl; > + int *num = &hapd->conf->num_deny_mac; > + int vlan_id = 0; /* TODO: Add VLAN support */ > + int rem = 0; > + int ret; > + > + wpa_dbg(hapd->msg_ctx, MSG_DEBUG, "DENY_STA: %s", txtaddr); > + > + if (txtaddr[0] == '-') { > + rem = 1; > + txtaddr++; > + } > + else if (txtaddr[0] == '+') { > + rem = 0; > + txtaddr++; > + } > + > + if (hwaddr_aton(txtaddr, addr)) { > + ret = os_snprintf(buf, buflen, "BAD_ADDR\n"); > + if (os_snprintf_error(buflen, ret)) > + return 0; > + return ret; > + } > + > + if (rem) { > + int i = 0; > + while (i < *num) { > + if (os_memcmp((*acl)[i].addr, addr, ETH_ALEN) == 0) { > + os_remove_in_array(*acl, *num, sizeof(**acl), i); > + (*num)--; > + } else > + i++; > + } > + } > + else { > + newacl = os_realloc_array(*acl, *num + 1, sizeof(**acl)); > + if (newacl == NULL) { > + ret = os_snprintf(buf, buflen, "NO_MEM\n"); > + if > (os_snprintf_error(buflen, ret)) > + return 0; > + return ret; > + } > + > + *acl = newacl; > + os_memcpy((*acl)[*num].addr, addr, ETH_ALEN); > + os_memset(&(*acl)[*num].vlan_id, 0, > + sizeof((*acl)[*num].vlan_id)); > + (*acl)[*num].vlan_id.untagged = vlan_id; > + (*acl)[*num].vlan_id.notempty = !!vlan_id; > + (*num)++; > + } > + > + qsort(*acl, *num, sizeof(**acl), hostapd_acl_comp); > + > + return 0; > + > +} > + > + > +int hostapd_ctrl_iface_accept_sta(struct hostapd_data *hapd, const char *txtaddr, > + char *buf, size_t buflen) > +{ > + u8 addr[ETH_ALEN]; > + struct mac_acl_entry **acl = &hapd->conf->accept_mac; > + struct mac_acl_entry *newacl; > + int *num = &hapd->conf->num_accept_mac; > + int vlan_id = 0; /* TODO: Add VLAN support */ > + int rem = 0; > + int ret; > + > + wpa_dbg(hapd->msg_ctx, MSG_DEBUG, > "ACCEPT_STA: %s", txtaddr); > + > + if (txtaddr[0] == '-') { > + rem = 1; > + txtaddr++; > + } > + else if (txtaddr[0] == '+') { > + rem = 0; > + txtaddr++; > + } > + > + if (hwaddr_aton(txtaddr, addr)) { > + ret = os_snprintf(buf, buflen, "BAD_ADDR\n"); > + if (os_snprintf_error(buflen, ret)) > + return 0; > + return ret; > + } > + > + if (rem) { > + int i = 0; > + while (i < *num) { > + if (os_memcmp((*acl)[i].addr, addr, ETH_ALEN) == 0) { > + os_remove_in_array(*acl, *num, sizeof(**acl), i); > + (*num)--; > + } else > + i++; > + } > + } > + else { > + newacl = > os_realloc_array(*acl, *num + 1, sizeof(**acl)); > + if (newacl == NULL) { > + ret = os_snprintf(buf, buflen, "NO_MEM\n"); > + if (os_snprintf_error(buflen, ret)) > + return 0; > + return ret; > + } > + > + *acl = newacl; > + os_memcpy((*acl)[*num].addr, addr, ETH_ALEN); > + > os_memset(&(*acl)[*num].vlan_id, 0, > + sizeof((*acl)[*num].vlan_id)); > + (*acl)[*num].vlan_id.untagged = vlan_id; > + (*acl)[*num].vlan_id.notempty = !!vlan_id; > + (*num)++; > + } > + > + qsort(*acl, *num, sizeof(**acl), hostapd_acl_comp); > + > + return 0; > + > +} > + > + > #ifdef CONFIG_P2P_MANAGER > static int p2p_manager_disconnect(struct hostapd_data *hapd, u16 stype, > u8 minor_reason_code, const u8 *addr) > diff --git a/src/ap/ctrl_iface_ap.h b/src/ap/ctrl_iface_ap.h > index 4f99680..b662922 100644 > --- a/src/ap/ctrl_iface_ap.h > +++ b/src/ap/ctrl_iface_ap.h > @@ -15,6 +15,10 @@ int hostapd_ctrl_iface_sta(struct hostapd_data *hapd, const char *txtaddr, > char *buf, size_t buflen); > int hostapd_ctrl_iface_sta_next(struct hostapd_data *hapd, const char *txtaddr, > char *buf, size_t buflen); > +int hostapd_ctrl_iface_deny_sta(struct hostapd_data *hapd, const char *txtaddr, > + char *buf, size_t buflen); > +int hostapd_ctrl_iface_accept_sta(struct hostapd_data *hapd, const char *txtaddr, > + char *buf, size_t buflen); > int hostapd_ctrl_iface_deauthenticate(struct hostapd_data *hapd, > const char *txtaddr); > int hostapd_ctrl_iface_disassociate(struct hostapd_data *hapd, > _______________________________________________ Hostap mailing list Hostap@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/hostap
