ACL Bug: MAC Address Bitmasks Ignored After VLAN ID

[s0lst1c3]

Description

When a user attempts to use an ACL file containing VLAN IDs, hostapd-mana ignores all bitmasks after the first line containing a VLAN ID unless the line also has a VLAN ID.

For example, the bitmasks for the last three entries in the following hostapd.accept file would be ignored:

To Reproduce

Start hostapd-mana using the following configuration file and hostapd.accept file (make sure to enable debug output using the -d flag):

hostapd.conf

enable_mana=0
mana_loud=0
mana_macacl=1
interface=wlan0
logger_syslog=-1
logger_syslog_level=2
logger_stdout=-1
logger_stdout_level=2
ssid=testnetwork
hw_mode=g
channel=1
macaddr_acl=1
accept_mac_file=./hostapd.accept
ignore_broadcast_ssid=0

hostapd.accept

# List of MAC addresses that are allowed to authenticate (IEEE 802.11)
# with the AP. Optional VLAN ID can be assigned for clients based on the
# MAC address if dynamic VLANs (hostapd.conf dynamic_vlan option) are used.
00:11:22:33:44:55 00:ff:ff:ff:ff:ff
00:66:77:88:99:aa 1
a4:83:e7:02:1a:9c 00:00:00:ff:ff:ff

Make sure to use an external WiFi adapter rather than mac80211_hwsim, since mac80211_hwsim cycles through MAC addresses fairly quickly.

Set client MAC address to a4:83:e7:02:1a:9c and attempt to connect to testnetwork.

Observe the following line in hostapd's debug output:

Cause of Issue

In the hostapd_config_read_maclist() function found within hostapd/config_file.c (lines 184-320), the variable vlanflag is not reset to 0 after each iteration of the while loop found on line 206.

Detailed Walkthrough

We first store the first mac address on line 17 (whitelisted mac address) in the variable addr:

At this time, pos is pointing to the character highlighted by the cursor in the following screenshot:

Next, we reach the following block of code, which is skipped since our hostapd.accept file does not flag any addresses for removal:

Next, we point pos to the beginning of our buffer buf (see previous screenshot) and assume our vlan_id is 0 (the default):

We then increment pointer pos until it points to a character that is not a tab, not a space, and not a null terminator:

This causes pos to point to the location shown in the screenshot below:

Next, we keep incrementing pos until it's no longer pointing at a space or tab:

This causes pos to point to the vlan ID at the end of the line, as shown in the following screenshot:

We then check to see if we've reached the end of the line (by checking if pos is pointing to a null terminator). If it's not, we attempt to determine whether pos pointing to the beginning of a mac address or whether pos is pointing to a VLAN flag. If pos is pointing to a VLAN flag, we store the VLAN flag in vlan_id and set vlanflag to TRUE.

Next, we set replace the newline character at the end of the string with a null terminator:

At this point, pos is still pointing to the beginning of our VLAN ID. The next block of code checks to see if vlanflag is set to TRUE. Since we set vlanflag to TRUE earlier, we then increment pos until it's pointing to either a null terminator or the start of a mac address (whichever comes first):

Pos is now pointing to the null terminator at the end of our line buffer (I'm depicting this as best I can using vim):

The next block of code determines whether pos is pointing to the end of the line or whether it's pointing the beginning of a mac address mask. In the latter case, it parses the mac address mask and stores it in mask. In the former case, it sets mask to ff:ff:ff:ff:ff:ff (exact match).

A bunch of other stuff then happens, but it's not relevant. We proceed to the next iteration and read the next line of the file.

Here's the problem: we never reset vlanflag to FALSE before the next iteration.

The next time we reach the following block of code...

... pos will be pointing at the beginning of the mac address mask on line 18:

Since vlanflag is still set to TRUE, pos is then incremented to the end of the line, which causes the following block of code to set mask to the default ff:ff:ff:ff:ff:ff instead of the mask shown in the hostapd.accept file:

The result is that every subsequent bitmask in the hostapd.accept file is ignored (unless the line also contains a VLAN flag).