Allow remote RADIUS authentication with local VLAN management

The documentation in the hostapd.conf file says that the dynamic_vlan
variable is used to control whether VLAN assignments are accepted from a
RADIUS server. The implication seems to be that a static VLAN assignment
will come from the accept_mac_file if dynamic_vlan is set to 0, and a
dynamic assignment will come from the RADIUS server if dynamic_vlan is
set to 1. Instead, I'm seeing that the static settings from the
accept_mac_file are ignored if dynamic_vlan is set to 0, but used if
dynamic_vlan is set to 1. If dynamic_vlan is set to 1 and the RADIUS
server does not provide a VLAN, then the accept_mac_file assignment is
overridden and the STA is assigned to the default non-VLANed interface.

If my understanding of the expected behavior is correct, then I believe
the problem is in ap_sta_set_vlan(). That routine checks the
dynamic_vlan setting, but has no way of determining whether the incoming
vlan_desc is static (i.e., from accept_mac_file) or dynamic (i.e., from
a RADIUS server).

I've attached a patch that gets hostapd working as I believe it's meant
to, and updates the documentation to make the implicit behavior
explicit.

The functional changes are:

- hostapd_allowed_address() will always extract the vlan_id from the
  accept_macs file. It will not update the vlan_id from the RADIUS cache
  if dynamic_vlan is DISABLED.

- hostapd_acl_recv_radius() will not update the cached vlan_id if
  dynamic_vlan is DISABLED.

- ieee802_1x_receive_auth() will not update the vlan_id if dynamic_vlan
  is DISABLED.

More cosmetic:

Most of the delta is just moving code out of ieee802_1x_receive_auth()
into a new ieee802_1x_update_vlan() routine. While I initially did this
because the new DISABLED check introduced excessive indentation, it has
the added advantage of eliminating the vlan_description allocation and
os_memset() call for all DYNAMIC_VLAN_DISABLED configs.

I've done a couple rounds of review offline with Michael Braun (who has
done much of the work in this part of the code) and incorporated his
feedback.

If dynamic_vlan=0 (disabled), vlan assignments will be managed using the
local accept_mac_file ACL file, even if a RADIUS server is being used
for user authentication. This allows us to manage users and devices
independently.

Signed-off-by: Nils Nieuwejaar <nils.nieuwejaar@gmail.com>

hostapd/hostapd.conf

@@ -1118,8 +1118,8 @@ own_ip_addr=127.0.0.1
 # Tunnel-Medium-Type (value 6 = IEEE 802), Tunnel-Private-Group-ID (value
 # VLANID as a string). Optionally, the local MAC ACL list (accept_mac_file) can
 # be used to set static client MAC address to VLAN ID mapping.
-# 0 = disabled (default)
+# 0 = disabled (default); only VLAN IDs from accept_mac_file will be used
-# 1 = option; use default interface if RADIUS server does not include VLAN ID
+# 1 = optional; use default interface if RADIUS server does not include VLAN ID
 # 2 = required; reject authentication if RADIUS server does not include VLAN ID
 #dynamic_vlan=0
 

src/ap/ieee802_11_auth.c

@@ -289,6 +289,9 @@ int hostapd_allowed_address(struct hostapd_data *hapd, const u8 *addr,
 			return HOSTAPD_ACL_ACCEPT;
 		};
 
+		if (hapd->conf->ssid.dynamic_vlan == DYNAMIC_VLAN_DISABLED)
+			vlan_id = NULL;
+
 		/* Check whether ACL cache has an entry for this station */
 		res = hostapd_acl_cache_get(hapd, addr, session_timeout,
 					    acct_interim_interval, vlan_id, psk,
@@ -516,7 +519,6 @@ hostapd_acl_recv_radius(struct radius_msg *msg, struct radius_msg *req,
 	struct hostapd_acl_query_data *query, *prev;
 	struct hostapd_cached_radius_acl *cache;
 	struct radius_hdr *hdr = radius_msg_get_hdr(msg);
-	int *untagged, *tagged, *notempty;
 
 	query = hapd->acl_queries;
 	prev = NULL;
@@ -574,12 +576,10 @@ hostapd_acl_recv_radius(struct radius_msg *msg, struct radius_msg *req,
 			cache->acct_interim_interval = 0;
 		}
 
-		notempty = &cache->vlan_id.notempty;
+		if (hapd->conf->ssid.dynamic_vlan != DYNAMIC_VLAN_DISABLED)
-		untagged = &cache->vlan_id.untagged;
+			cache->vlan_id.notempty = !!radius_msg_get_vlanid(
-		tagged = cache->vlan_id.tagged;
+				msg, &cache->vlan_id.untagged,
-		*notempty = !!radius_msg_get_vlanid(msg, untagged,
+				MAX_NUM_TAGGED_VLAN, cache->vlan_id.tagged);
-						    MAX_NUM_TAGGED_VLAN,
-						    tagged);
 
 		decode_tunnel_passwords(hapd, shared_secret, shared_secret_len,
 					msg, req, cache);

src/ap/ieee802_1x.c

@@ -1742,6 +1742,45 @@ ieee802_1x_search_radius_identifier(struct hostapd_data *hapd, u8 identifier)
 }
 
 
+#ifndef CONFIG_NO_VLAN
+static int ieee802_1x_update_vlan(struct radius_msg *msg,
+				  struct hostapd_data *hapd,
+				  struct sta_info *sta)
+{
+	struct vlan_description vlan_desc;
+
+	os_memset(&vlan_desc, 0, sizeof(vlan_desc));
+	vlan_desc.notempty = !!radius_msg_get_vlanid(msg, &vlan_desc.untagged,
+						     MAX_NUM_TAGGED_VLAN,
+						     vlan_desc.tagged);
+
+	if (vlan_desc.notempty &&
+	    !hostapd_vlan_valid(hapd->conf->vlan, &vlan_desc)) {
+		sta->eapol_sm->authFail = TRUE;
+		hostapd_logger(hapd, sta->addr, HOSTAPD_MODULE_RADIUS,
+			       HOSTAPD_LEVEL_INFO,
+			       "Invalid VLAN %d%s received from RADIUS server",
+			       vlan_desc.untagged,
+			       vlan_desc.tagged[0] ? "+" : "");
+		os_memset(&vlan_desc, 0, sizeof(vlan_desc));
+		ap_sta_set_vlan(hapd, sta, &vlan_desc);
+		return -1;
+	}
+
+	if (hapd->conf->ssid.dynamic_vlan == DYNAMIC_VLAN_REQUIRED &&
+	    !vlan_desc.notempty) {
+		sta->eapol_sm->authFail = TRUE;
+		hostapd_logger(hapd, sta->addr, HOSTAPD_MODULE_IEEE8021X,
+			       HOSTAPD_LEVEL_INFO,
+			       "authentication server did not include required VLAN ID in Access-Accept");
+		return -1;
+	}
+
+	return ap_sta_set_vlan(hapd, sta, &vlan_desc);
+}
+#endif /* CONFIG_NO_VLAN */
+
+
 /**
  * ieee802_1x_receive_auth - Process RADIUS frames from Authentication Server
  * @msg: RADIUS response message
@@ -1764,12 +1803,6 @@ ieee802_1x_receive_auth(struct radius_msg *msg, struct radius_msg *req,
 	struct eapol_state_machine *sm;
 	int override_eapReq = 0;
 	struct radius_hdr *hdr = radius_msg_get_hdr(msg);
-	struct vlan_description vlan_desc;
-#ifndef CONFIG_NO_VLAN
-	int *untagged, *tagged, *notempty;
-#endif /* CONFIG_NO_VLAN */
-
-	os_memset(&vlan_desc, 0, sizeof(vlan_desc));
 
 	sm = ieee802_1x_search_radius_identifier(hapd, hdr->identifier);
 	if (sm == NULL) {
@@ -1834,56 +1867,21 @@ ieee802_1x_receive_auth(struct radius_msg *msg, struct radius_msg *req,
 	switch (hdr->code) {
 	case RADIUS_CODE_ACCESS_ACCEPT:
 #ifndef CONFIG_NO_VLAN
-		if (hapd->conf->ssid.dynamic_vlan != DYNAMIC_VLAN_DISABLED) {
+		if (hapd->conf->ssid.dynamic_vlan != DYNAMIC_VLAN_DISABLED &&
-			notempty = &vlan_desc.notempty;
+		    ieee802_1x_update_vlan(msg, hapd, sta) < 0)
-			untagged = &vlan_desc.untagged;
-			tagged = vlan_desc.tagged;
-			*notempty = !!radius_msg_get_vlanid(msg, untagged,
-							    MAX_NUM_TAGGED_VLAN,
-							    tagged);
-		}
-
-		if (vlan_desc.notempty &&
-		    !hostapd_vlan_valid(hapd->conf->vlan, &vlan_desc)) {
-			sta->eapol_sm->authFail = TRUE;
-			hostapd_logger(hapd, sta->addr,
-				       HOSTAPD_MODULE_RADIUS,
-				       HOSTAPD_LEVEL_INFO,
-				       "Invalid VLAN %d%s received from RADIUS server",
-				       vlan_desc.untagged,
-				       vlan_desc.tagged[0] ? "+" : "");
-			os_memset(&vlan_desc, 0, sizeof(vlan_desc));
-			ap_sta_set_vlan(hapd, sta, &vlan_desc);
-			break;
-		}
-
-		if (hapd->conf->ssid.dynamic_vlan == DYNAMIC_VLAN_REQUIRED &&
-		    !vlan_desc.notempty) {
-			sta->eapol_sm->authFail = TRUE;
-			hostapd_logger(hapd, sta->addr,
-				       HOSTAPD_MODULE_IEEE8021X,
-				       HOSTAPD_LEVEL_INFO, "authentication "
-				       "server did not include required VLAN "
-				       "ID in Access-Accept");
-			break;
-		}
-#endif /* CONFIG_NO_VLAN */
-
-		if (ap_sta_set_vlan(hapd, sta, &vlan_desc) < 0)
 			break;
 
-#ifndef CONFIG_NO_VLAN
 		if (sta->vlan_id > 0) {
 			hostapd_logger(hapd, sta->addr,
 				       HOSTAPD_MODULE_RADIUS,
 				       HOSTAPD_LEVEL_INFO,
 				       "VLAN ID %d", sta->vlan_id);
 		}
-#endif /* CONFIG_NO_VLAN */
 
 		if ((sta->flags & WLAN_STA_ASSOC) &&
 		    ap_sta_bind_vlan(hapd, sta) < 0)
 			break;
+#endif /* CONFIG_NO_VLAN */
 
 		sta->session_timeout_set = !!session_timeout_set;
 		os_get_reltime(&sta->session_timeout);

src/ap/sta_info.c

@@ -897,9 +897,6 @@ int ap_sta_set_vlan(struct hostapd_data *hapd, struct sta_info *sta,
 	struct hostapd_vlan *vlan = NULL, *wildcard_vlan = NULL;
 	int old_vlan_id, vlan_id = 0, ret = 0;
 
-	if (hapd->conf->ssid.dynamic_vlan == DYNAMIC_VLAN_DISABLED)
-		vlan_desc = NULL;
-
 	/* Check if there is something to do */
 	if (hapd->conf->ssid.per_sta_vif && !sta->vlan_id) {
 		/* This sta is lacking its own vif */