From patchwork Fri Jan 26 20:01:54 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Nils Nieuwejaar X-Patchwork-Id: 866722 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.infradead.org (client-ip=65.50.211.133; helo=bombadil.infradead.org; envelope-from=hostap-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="i0VGorZb"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="ZV2v2GzN"; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3zT8t62QGgz9sNV for ; Sat, 27 Jan 2018 20:02:22 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:MIME-Version :From:Reply-To:Cc:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=sRO1c5F9pL6Kxcqx96Zy1ibT9KcRWn3ikkjrdJzlEYo=; b=i0VGorZbM/nrRA kUsJL4TMHggb52B6581w8sG7ipuNdIisft+Zr5/KHsuUrkluVEOit7VeRRIOtnZ+CT2AmSa3br5B4 4cDjku9BvDQWPNBrMQexFYADC1S2by7yyY7MP2KPBq8cwnYuyT5i1O1xf+Sm/8Qj1vxDEl/8HJ9WS CVyNpkR/wmysgiVoWRIAbksK6s3fVngFhTyL0lwqnHv63AJWubzJzMysT7C5Xv7GmxsOfMaZkv5Ca Djez1zL+TorAL0+n+kWM+6NeP6cZlejNbBb2Vk+ehx3oxvA3VtkzsBylVFQarZGAZW5QrwkD7ToBG bYbprGOJ3yEKTDiUcQOw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.89 #1 (Red Hat Linux)) id 1efMMp-0008HC-J2; Sat, 27 Jan 2018 09:01:43 +0000 Received: from mail-io0-x231.google.com ([2607:f8b0:4001:c06::231]) by bombadil.infradead.org with esmtps (Exim 4.89 #1 (Red Hat Linux)) id 1efACL-0003dL-Kc for hostap@lists.infradead.org; Fri, 26 Jan 2018 20:02:07 +0000 Received: by mail-io0-x231.google.com with SMTP id f4so1621716ioh.8 for ; Fri, 26 Jan 2018 12:01:55 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:mime-version:date:message-id:subject:to :content-transfer-encoding; bh=1+FHy0oDY646fckbYIl3wVJ3psAfQcDz1Qm2ECrAYuU=; b=ZV2v2GzNZ2lRIRmxCCj24McdCD2g3ZdFnsG2GM9RGsyCC8J3DGrEN/TLICCGEEtxXg naxzbmrI1TmF7TIFDMrOfg48LJZhkvBB6J84yhtq8uNwm6GIUcHU8cbzyomvbED3tRZX pHKO8p3plYj+/+guLMSiXaPF4dN3NA5SGNZx239WLxSgcTXJFFM2W4OUHUE8By34Kr+c CRTi7bZ2AUI7G+tZzLOMXrmZUYXUEMnUBPR+01zsnzRLyXx0bHfWCVrGeM2tKMD/05l7 lDHVWZj+Rfx+cBpNkaJwl4tfzVp6Uyt1B6qOmAOCUoLNhRyiKdl65m3VqVhFHQsljozM sPcA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:mime-version:date:message-id:subject:to :content-transfer-encoding; bh=1+FHy0oDY646fckbYIl3wVJ3psAfQcDz1Qm2ECrAYuU=; b=nFMQ2hsIWhQrYCFY/QoPxDkffa0aPXl+F3gvGwtU9XAZBJb1AaZ8cTPA4RSDToPPlt 1K9uGijZN5OJcEjXsdQLtGcPTrYGuVCiUSjTliphTMO+8LgIY8yyMmHWNZY4xlHFvVWa 64BvSfAj1m3Bw52Y/G8+gk+C8AUFbVXw0DioWw6CQGk7D2aQ5JCPvxtxuy/5MPJH/RYI oeJeOOzQT2QJqYyGniZfxhvt4kEVWOXeXxUYbNTk9m8xw8M97OdeCDlNnY09IwbY7En5 yLhblS2PtJWjOJbfvltATyKKUqbik301QdzgkUTZZP11cFMbNFBtp21G1k/WWP56tRGU D8Wg== X-Gm-Message-State: AKwxytcNHMM8fDehlyqOw2OgTqC2rVH1TK5lKWioRiCQuhNL9Fw7LStY xRZzDaoqc7Xx3jVj2YOnhk4FILCbawhgm05SLtSj0Q== X-Google-Smtp-Source: AH8x227LP5pjLuY6svXo1F+DdpWxSAZGZErFJNE6EyOHmNXNGPuIKryo5XDVDtEvtXWTyVOszHPsx3VtvwGg5Kj0ago= X-Received: by 10.107.17.27 with SMTP id z27mr18333900ioi.254.1516996914488; Fri, 26 Jan 2018 12:01:54 -0800 (PST) Received: from 1058052472880 named unknown by gmailapi.google.com with HTTPREST; Fri, 26 Jan 2018 15:01:54 -0500 From: Nils Nieuwejaar X-Mailer: Airmail (467) MIME-Version: 1.0 Date: Fri, 26 Jan 2018 15:01:54 -0500 Message-ID: Subject: VLANs, accept_mac_file, and RADIUS To: hostap@lists.infradead.org X-Spam-Note: CRM114 invocation failed X-Spam-Score: -2.7 (--) X-Spam-Report: SpamAssassin version 3.4.1 on bombadil.infradead.org summary: Content analysis details: (-2.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RCVD_IN_DNSWL_LOW RBL: Sender listed at http://www.dnswl.org/, low trust [2607:f8b0:4001:c06:0:0:0:231 listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (nils.nieuwejaar[at]gmail.com) -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid 0.0 UNPARSEABLE_RELAY Informational: message has unparseable relay lines X-Mailman-Approved-At: Sat, 27 Jan 2018 01:01:40 -0800 X-BeenThere: hostap@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Hostap" Errors-To: hostap-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org Hi, In my environment, I am using WPA-EAP to authenticate users, but I want the VLAN assignment to come from the accept_mac_file rather than the RADIUS server. I'm happy to get into my reasons for that if you care, but I don't think it really matters. 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. Nils   /* This sta is lacking its own vif */ diff --git a/hostapd/hostapd.conf b/hostapd/hostapd.conf index 73e8fc39f..5401a3831 100644 --- a/hostapd/hostapd.conf +++ b/hostapd/hostapd.conf @@ -1104,7 +1104,7 @@ 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  # 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 3308398d1..c294c67cc 100644 --- a/src/ap/ieee802_11_auth.c +++ b/src/ap/ieee802_11_auth.c @@ -281,6 +281,9 @@ int hostapd_allowed_address(struct hostapd_data *hapd, const u8 *addr,  #else /* CONFIG_NO_RADIUS */   struct hostapd_acl_query_data *query; + 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, @@ -566,12 +569,14 @@ 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) { + 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); + }   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 793d381ed..ffe539a4d 100644 --- a/src/ap/ieee802_1x.c +++ b/src/ap/ieee802_1x.c @@ -1673,6 +1673,49 @@ 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; + int *untagged, *tagged, *notempty; + + os_memset(&vlan_desc, 0, sizeof(vlan_desc)); + 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); + 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 @@ -1694,12 +1737,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) { @@ -1764,56 +1801,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;   sta->session_timeout = session_timeout; diff --git a/src/ap/sta_info.c b/src/ap/sta_info.c index d4f00d1f9..b9d4b49e4 100644 --- a/src/ap/sta_info.c +++ b/src/ap/sta_info.c @@ -888,9 +888,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) {