From ba4f3224ae9e3b8005296123d5672fefbe0d7267 Mon Sep 17 00:00:00 2001 From: Nils Nieuwejaar Date: Wed, 30 May 2018 14:09:01 -0700 Subject: [PATCH] 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 --- hostapd/hostapd.conf | 4 +- src/ap/ieee802_11_auth.c | 14 +++---- src/ap/ieee802_1x.c | 86 ++++++++++++++++++++-------------------- src/ap/sta_info.c | 3 -- 4 files changed, 51 insertions(+), 56 deletions(-) diff --git a/hostapd/hostapd.conf b/hostapd/hostapd.conf index d234a4339..d7add2df1 100644 --- 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 5cb7fb145..931d4d065 100644 --- 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 68d3a81a1..a56c82e56 100644 --- 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); - 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) + if (hapd->conf->ssid.dynamic_vlan != DYNAMIC_VLAN_DISABLED && + ieee802_1x_update_vlan(msg, hapd, sta) < 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 217ca6351..06f9afcd7 100644 --- 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 */