[PATCHv2] hostapd: reload bss mac files on SIGUSR2

[Jouni Malinen]

On Mon, Jun 11, 2012 at 12:44:30AM +0200, Antonio Quartulli wrote:
> This patch allows the administrator to modify the provided MAC list for the
> BSS ACL mechanism at runtime. In the current implementation no runtime change
> is possible: if the MAC list is modified, hostapd has to be restart so
> disconnecting all the current clients.
> 
> After manually modify the MAC files, the USR2 signal will make hostapd reload
> them and possibly disconnect new not allowed stations.

Why is this needed? Doesn't SIGHUP already reload the files? Using a
USR2 signal would require more justification since it is the only
generic signal that remains available and I'm not sure whether this
functionality is really in need of a global signal (i.e., per-interface
ctrl_iface would be cleaner).

> diff --git a/hostapd/config_file.c b/hostapd/config_file.c
> @@ -1391,8 +1394,12 @@ static int hostapd_config_fill(struct hostapd_config *conf,
>  				wpa_printf(MSG_ERROR, "Line %d: unknown "
>  					   "macaddr_acl %d",
>  					   line, bss->macaddr_acl);
> +				bss->accept_mac_filename[0] = '\0';
> +				bss->deny_mac_filename[0] = '\0';
>  			}
>  		} else if (os_strcmp(buf, "accept_mac_file") == 0) {
> +			os_strncpy(bss->accept_mac_filename, pos, 256);
> +			bss->accept_mac_filename[255] = '\0';

os_strlcpy() should be used instead of those two calls.

> +int hostapd_reload_macfile(struct hostapd_iface *iface)
...
> +		/* reload accept mac file if any */
> +		if (bss->accept_mac_filename[0] != '\0') {
> +			wpa_printf(MSG_DEBUG, "Reading %s",
> +				   bss->accept_mac_filename);
> +			os_free(bss->accept_mac);
> +			bss->accept_mac = NULL;
> +			bss->num_accept_mac = 0;
> +			if (hostapd_config_read_maclist(
> +						bss->accept_mac_filename,
> +						&bss->accept_mac,
> +						&bss->num_accept_mac))
> +				wpa_printf(MSG_ERROR, "Failed to read "
> +					   "accept_mac_file '%s'",
> +					   bss->accept_mac_filename);
> +		}

Will that error leave hostapd running with empty MAC ACL list? If so, it
would be better to either leave the old list in use or prevent the use
of the BSS after such failure.

> +		/* deauthenticate listed stations */
> +		if (bss->macaddr_acl == ACCEPT_UNLESS_DENIED)
> +			for (j = 0; j < bss->num_deny_mac; j++)
> +				hostapd_deauth_station(iface->bss[i],
> +					bss->deny_mac[j].addr,
> +					WLAN_REASON_PREV_AUTH_NOT_VALID);

Is this sending Deauthentication frame to every MAC address that is in
the deny list even if those STAs are not authenticated at the moment?
That would expose the list by broadcasting it.. This does not sound
reasonable, i.e., only the STAs that are actually authenticated should
be deauthenticated.

> diff --git a/src/ap/ap_config.h b/src/ap/ap_config.h
> @@ -207,14 +207,15 @@ struct hostapd_bss_config {
>  	int ieee802_11f; /* use IEEE 802.11f (IAPP) */
>  	char iapp_iface[IFNAMSIZ + 1]; /* interface used with IAPP broadcast
>  					* frames */
> -
>  	enum {

Please do not include unrelated changes..

> diff --git a/src/ap/hostapd.c b/src/ap/hostapd.c
> @@ -106,7 +106,6 @@ static void hostapd_reload_bss(struct hostapd_data *hapd)
>  	wpa_printf(MSG_DEBUG, "Reconfigured interface %s", hapd->conf->iface);
>  }
>  
> -
>  int hostapd_reload_config(struct hostapd_iface *iface)

... same here..

> diff --git a/src/ap/sta_info.c b/src/ap/sta_info.c
> +void hostapd_free_sta(struct hostapd_data *hapd, u8 *addr, u16 reason)
> +{
> +	struct sta_info *sta, *prev;
> +
> +	prev = sta = hapd->sta_list;
> +
> +	while (sta && memcmp(sta->addr, addr, ETH_ALEN)) {
> +		prev = sta;
> +		sta = sta->next;
> +	}

Should use ap_get_sta() instead of another loop to go through the STA
list.

> +	if (sta == hapd->sta_list)
> +		hapd->sta_list = sta->next;
> +	else
> +		prev->next = sta->next;
> +
> +	hostapd_free_sta_info(hapd, sta, reason);

Why are the STA list pointers modified here instead of just leaving this
to ap_free_sta() -> ap_sta_list_del() that gets called from
hostapd_free_sta_info()?

-- 
Jouni Malinen                                            PGP id EFC895FA