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>

diff --git a/hostapd/hostapd.conf b/hostapd/hostapd.conf
index d234a43399486a39ccfdd2339ecc2ac23831ca1b..d7add2df15fc8fbefbc8b496c7e3c9527d0cc0e6 100644 (file)
--- a/hostapd/hostapd.conf
+++ b/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)
-# 1 = option; use default interface if RADIUS server does not include VLAN ID
+# 0 = disabled (default); only VLAN IDs from accept_mac_file will be used
+# 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
 

diff --git a/src/ap/ieee802_11_auth.c b/src/ap/ieee802_11_auth.c
index 5cb7fb1454f769d6a85446f08dcce17f4feb6ccf..931d4d0659c3c9e7d3872235685ac831740559f0 100644 (file)
--- a/src/ap/ieee802_11_auth.c
+++ b/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;
-               untagged = &cache->vlan_id.untagged;
-               tagged = cache->vlan_id.tagged;
-               *notempty = !!radius_msg_get_vlanid(msg, untagged,
-                                                   MAX_NUM_TAGGED_VLAN,
-                                                   tagged);
+               if (hapd->conf->ssid.dynamic_vlan != DYNAMIC_VLAN_DISABLED)
+                       cache->vlan_id.notempty = !!radius_msg_get_vlanid(
+                               msg, &cache->vlan_id.untagged,
+                               MAX_NUM_TAGGED_VLAN, cache->vlan_id.tagged);
 
                decode_tunnel_passwords(hapd, shared_secret, shared_secret_len,
                                        msg, req, cache);

diff --git a/src/ap/ieee802_1x.c b/src/ap/ieee802_1x.c
index 68d3a81a16ad19d0efeb230ada8e44147cb5e7f0..a56c82e564e63220aef78456b876111466cf0a5b 100644 (file)
--- a/src/ap/ieee802_1x.c
+++ b/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) {
-                       notempty = &vlan_desc.notempty;
-                       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);
+               if (hapd->conf->ssid.dynamic_vlan != DYNAMIC_VLAN_DISABLED &&
+                   ieee802_1x_update_vlan(msg, hapd, sta) < 0)
                        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);

diff --git a/src/ap/sta_info.c b/src/ap/sta_info.c
index 217ca635166d9cf63c8da3a261b89a4d4d639cde..06f9afcd713e5c12f945c2d949f1631b3122a224 100644 (file)
--- a/src/ap/sta_info.c
+++ b/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 */