ap: Refactor DHCP settings loading

Extend the [IPv4].Address setting's syntax to allow a new format: a list
of <IP>/<prefix_len> -form strings that define the address space from
which a subnet is selected.  Rewrite the DHCP settings loading with
other notable changes:

 * validate some of the settings more thoroughly,
 * name all netconfig-related ap_state members with the netconfig_
   prefix,
 * make sure we always call l_dhcp_server_set_netmask(),
 * allow netmasks other than 24-bit and change the default to 28 bits,
 * as requested avoid using the l_net_ ioctl-based functions although
   l_dhcp still uses them internally,
 * as requested avoid touching the ap_state members until the end of
   some functions so that on error they're basically a no-op (for
   readability).

src/ap.c

@@ -52,6 +52,7 @@
 #include "src/frame-xchg.h"
 #include "src/wscutil.h"
 #include "src/eap-wsc.h"
+#include "src/ip-pool.h"
 #include "src/ap.h"
 #include "src/storage.h"
 #include "src/diagnostic.h"
@@ -88,15 +89,15 @@ struct ap_state {
 	uint16_t last_aid;
 	struct l_queue *sta_states;
 
-	struct l_dhcp_server *server;
+	struct l_dhcp_server *netconfig_dhcp;
+	char *netconfig_addr4_str;
+	uint8_t netconfig_prefix_len4;
 	uint32_t rtnl_add_cmd;
-	char *own_ip;
-	unsigned int ip_prefix;
 
 	bool started : 1;
 	bool gtk_set : 1;
-	bool cleanup_ip : 1;
+	bool netconfig_set_addr4 : 1;
-	bool use_ip_pool : 1;
+	bool netconfig_use_ip_pool : 1;
 };
 
 struct sta_state {
@@ -181,7 +182,7 @@ static char *ip_pool_get()
 
 	/* This shouldn't happen */
 	if (next_subnet < pool.sub_start || next_subnet > pool.sub_end)
-		return NULL;
+		return 0;
 
 	l_uintset_put(pool.used, next_subnet);
 
@@ -321,23 +322,27 @@ static void ap_reset(struct ap_state *ap)
 	ap->started = false;
 
 	/* Delete IP if one was set by IWD */
-	if (ap->cleanup_ip)
+	if (ap->netconfig_set_addr4) {
 		l_rtnl_ifaddr4_delete(rtnl, netdev_get_ifindex(netdev),
-					ap->ip_prefix, ap->own_ip,
+				ap->netconfig_prefix_len4,
-					broadcast_from_ip(ap->own_ip),
+				ap->netconfig_addr4_str,
-					NULL, NULL, NULL);
+				broadcast_from_ip(ap->netconfig_addr4_str),
-
+				NULL, NULL, NULL);
-	if (ap->own_ip) {
+		ap->netconfig_set_addr4 = false;
-		/* Release IP from pool if used */
-		if (ap->use_ip_pool)
-			ip_pool_put(ap->own_ip);
-
-		l_free(ap->own_ip);
 	}
 
-	if (ap->server) {
+	if (ap->netconfig_use_ip_pool) {
-		l_dhcp_server_destroy(ap->server);
+		/* Release IP from pool if used */
-		ap->server = NULL;
+		ip_pool_put(ap->netconfig_addr4_str);
+		ap->netconfig_use_ip_pool = false;
+	}
+
+	l_free(ap->netconfig_addr4_str);
+	ap->netconfig_addr4_str = NULL;
+
+	if (ap->netconfig_dhcp) {
+		l_dhcp_server_destroy(ap->netconfig_dhcp);
+		ap->netconfig_dhcp = NULL;
 	}
 }
 
@@ -2165,7 +2170,7 @@ static void ap_start_cb(struct l_genl_msg *msg, void *user_data)
 		goto failed;
 	}
 
-	if (ap->server && !l_dhcp_server_start(ap->server)) {
+	if (ap->netconfig_dhcp && !l_dhcp_server_start(ap->netconfig_dhcp)) {
 		l_error("DHCP server failed to start");
 		goto failed;
 	}
@@ -2492,182 +2497,290 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
 	}
 }
 
-static bool dhcp_load_settings(struct ap_state *ap,
+#define AP_DEFAULT_IPV4_PREFIX_LEN 28
-				const struct l_settings *settings)
-{
-	struct l_dhcp_server *server = ap->server;
-	struct in_addr ia;
 
-	L_AUTO_FREE_VAR(char *, netmask) = l_settings_get_string(settings,
+static int ap_setup_netconfig4(struct ap_state *ap, const char **addr_str_list,
-							"IPv4", "Netmask");
+				uint8_t prefix_len, const char *gateway_str,
-	L_AUTO_FREE_VAR(char *, gateway) = l_settings_get_string(settings,
+				const char **ip_range,
-							"IPv4", "Gateway");
+				const char **dns_str_list,
-	char **dns = l_settings_get_string_list(settings, "IPv4",
+				unsigned int lease_time)
-							"DNSList", ',');
-	char **ip_range = l_settings_get_string_list(settings, "IPv4",
-							"IPRange", ',');
-	unsigned int lease_time;
-	bool ret = false;
-
-	if (!l_settings_get_uint(settings, "IPv4", "LeaseTime", &lease_time))
-		lease_time = 0;
-
-	if (gateway && !l_dhcp_server_set_gateway(server, gateway)) {
-		l_error("[IPv4].Gateway value error");
-		goto error;
-	}
-
-	if (dns && !l_dhcp_server_set_dns(server, dns)) {
-		l_error("[IPv4].DNSList value error");
-		goto error;
-	}
-
-	if (netmask && !l_dhcp_server_set_netmask(server, netmask)) {
-		l_error("[IPv4].Netmask value error");
-		goto error;
-	}
-
-	if (ip_range) {
-		if (l_strv_length(ip_range) != 2) {
-			l_error("Two addresses expected in [IPv4].IPRange");
-			goto error;
-		}
-
-		if (!l_dhcp_server_set_ip_range(server, ip_range[0],
-							ip_range[1])) {
-			l_error("Error setting IP range from [IPv4].IPRange");
-			goto error;
-		}
-	}
-
-	if (lease_time && !l_dhcp_server_set_lease_time(server, lease_time)) {
-		l_error("[IPv4].LeaseTime value error");
-		goto error;
-	}
-
-	if (netmask && inet_pton(AF_INET, netmask, &ia) > 0)
-		ap->ip_prefix = __builtin_popcountl(ia.s_addr);
-	else
-		ap->ip_prefix = 24;
-
-	ret = true;
-
-error:
-	l_strv_free(dns);
-	l_strv_free(ip_range);
-	return ret;
-}
-
-/*
- * This will determine the IP being used for DHCP. The IP will be automatically
- * set to ap->own_ip.
- *
- * The address to set (or keep) is determined in this order:
- * 1. Address defined in provisioning file
- * 2. Address already set on interface
- * 3. Address in IP pool.
- *
- * Returns:  0 if an IP was successfully selected and needs to be set
- *          -EALREADY if an IP was already set on the interface or
- *            IP configuration was not enabled,
- *          -EEXIST if the IP pool ran out of IP's
- *          -EINVAL if there was an error.
- */
-static int ap_setup_dhcp(struct ap_state *ap, const struct l_settings *settings)
 {
 	uint32_t ifindex = netdev_get_ifindex(ap->netdev);
+	struct l_rtnl_address *existing_addr = ip_pool_get_addr4(ifindex);
+	struct l_rtnl_address *new_addr = NULL;
+	int ret;
 	struct in_addr ia;
-	uint32_t address = 0;
+	struct l_dhcp_server *dhcp = NULL;
-	L_AUTO_FREE_VAR(char *, addr) = NULL;
+	bool use_ip_pool = false;
-	int ret = -EINVAL;
+	bool r;
+	char addr_str_buf[INET_ADDRSTRLEN];
 
-	/* get the current address if there is one */
+	dhcp = l_dhcp_server_new(ifindex);
-	if (l_net_get_address(ifindex, &ia) && ia.s_addr != 0)
+	if (!dhcp) {
-		address = ia.s_addr;
+		l_error("Failed to create DHCP server on ifindex %u", ifindex);
-
+		ret = -EIO;
-	addr = l_settings_get_string(settings, "IPv4", "Address");
+		goto cleanup;
-	if (addr) {
-		if (inet_pton(AF_INET, addr, &ia) < 0)
-			return -EINVAL;
-
-		/* Is a matching address already set on interface? */
-		if (ia.s_addr == address)
-			ret = -EALREADY;
-		else
-			ret = 0;
-	} else if (address) {
-		/* No address in config, but interface has one set */
-		addr = l_strdup(inet_ntoa(ia));
-		ret = -EALREADY;
-	} else if (pool.used) {
-		/* No config file, no address set. Use IP pool */
-		addr = ip_pool_get();
-		if (!addr) {
-			l_error("No more IP's in pool, cannot start AP on %u",
-					ifindex);
-			return -EEXIST;
-		}
-
-		ap->use_ip_pool = true;
-		ap->ip_prefix = pool.prefix;
-		ret = 0;
-	} else
-		return -EALREADY;
-
-	ap->server = l_dhcp_server_new(ifindex);
-	if (!ap->server) {
-		l_error("Failed to create DHCP server on %u", ifindex);
-		return -EINVAL;
 	}
 
 	if (getenv("IWD_DHCP_DEBUG"))
-		l_dhcp_server_set_debug(ap->server, do_debug,
+		l_dhcp_server_set_debug(dhcp, do_debug,
-							"[DHCPv4 SERV] ", NULL);
+					"[DHCPv4 SERV] ", NULL);
 
-	/* Set the remaining DHCP options in config file */
+	/*
-	if (!dhcp_load_settings(ap, settings))
+	 * The address pool specified for this AP (if any) has the priority,
-		return -EINVAL;
+	 * next is the address currently set on the interface (if any) and
+	 * last is the global AP address pool (APRanges setting).
+	 */
+	if (addr_str_list) {
+		if (!prefix_len)
+			prefix_len = AP_DEFAULT_IPV4_PREFIX_LEN;
 
-	if (!l_dhcp_server_set_ip_address(ap->server, addr))
+		ret = ip_pool_select_addr4(addr_str_list, prefix_len,
-		return -EINVAL;
+						&new_addr);
+	} else if (existing_addr &&
+			l_rtnl_address_get_prefix_length(existing_addr) <
+			31) {
+		if (!prefix_len)
+			prefix_len = l_rtnl_address_get_prefix_length(
+								existing_addr);
 
-	ap->own_ip = l_steal_ptr(addr);
+		if (!l_rtnl_address_get_address(existing_addr, addr_str_buf)) {
+			ret = -EIO;
+			goto cleanup;
+		}
+
+		new_addr = l_rtnl_address_new(addr_str_buf, prefix_len);
+		ret = 0;
+	} else {
+		L_AUTO_FREE_VAR(char *, addr_str) = NULL;
+
+		if (prefix_len) {
+			l_error("[IPv4].Netmask can't be used without an "
+				"address");
+			ret = -EINVAL;
+			goto cleanup;
+		}
+
+		/* No config, no address set. Use IP pool */
+		addr_str = ip_pool_get();
+		if (addr_str) {
+			use_ip_pool = true;
+			prefix_len = pool.prefix;
+			new_addr = l_rtnl_address_new(addr_str, prefix_len);
+			ret = 0;
+		} else {
+			l_error("No more IP's in pool, cannot start AP on %u",
+					ifindex);
+			ret = -EEXIST;
+		}
+	}
+
+	if (ret)
+		goto cleanup;
+
+	ret = -EIO;
+
+	/*
+	 * l_dhcp_server_start() would retrieve the current IPv4 from
+	 * the interface but set it anyway in case there are multiple
+	 * addresses, saves one ioctl too.
+	 */
+	if (!l_rtnl_address_get_address(new_addr, addr_str_buf)) {
+		l_error("l_rtnl_address_get_address failed");
+		goto cleanup;
+	}
+
+	if (!l_dhcp_server_set_ip_address(dhcp, addr_str_buf)) {
+		l_error("l_dhcp_server_set_ip_address failed");
+		goto cleanup;
+	}
+
+	ia.s_addr = htonl(util_netmask_from_prefix(prefix_len));
+
+	if (!l_dhcp_server_set_netmask(dhcp, inet_ntoa(ia))) {
+		l_error("l_dhcp_server_set_netmask failed");
+		goto cleanup;
+	}
+
+	if (gateway_str && !l_dhcp_server_set_gateway(dhcp, gateway_str)) {
+		l_error("l_dhcp_server_set_gateway failed");
+		goto cleanup;
+	}
+
+	if (ip_range) {
+		r = l_dhcp_server_set_ip_range(dhcp, ip_range[0], ip_range[1]);
+		if (!r) {
+			l_error("l_dhcp_server_set_ip_range failed");
+			goto cleanup;
+		}
+	}
+
+	if (dns_str_list) {
+		r = l_dhcp_server_set_dns(dhcp, (char **) dns_str_list);
+		if (!r) {
+			l_error("l_dhcp_server_set_dns failed");
+			goto cleanup;
+		}
+	}
+
+	if (lease_time && !l_dhcp_server_set_lease_time(dhcp, lease_time)) {
+		l_error("l_dhcp_server_set_lease_time failed");
+		goto cleanup;
+	}
+
+	ap->netconfig_addr4_str = l_strdup(addr_str_buf);
+	ap->netconfig_prefix_len4 = prefix_len;
+	ap->netconfig_set_addr4 = true;
+	ap->netconfig_use_ip_pool = use_ip_pool;
+	ap->netconfig_dhcp = l_steal_ptr(dhcp);
+	ret = 0;
+
+	if (existing_addr && l_rtnl_address_get_prefix_length(existing_addr) >
+			prefix_len) {
+		char addr_str_buf2[INET_ADDRSTRLEN];
+
+		if (l_rtnl_address_get_address(existing_addr, addr_str_buf2) &&
+				!strcmp(addr_str_buf, addr_str_buf2))
+			ap->netconfig_set_addr4 = false;
+	}
+
+cleanup:
+	l_dhcp_server_destroy(dhcp);
+	l_rtnl_address_free(new_addr);
+	l_rtnl_address_free(existing_addr);
 	return ret;
 }
 
-static int ap_load_dhcp(struct ap_state *ap, const struct l_settings *config,
+static int ap_load_ipv4(struct ap_state *ap, const struct l_settings *config)
-			bool *wait_dhcp)
 {
-	uint32_t ifindex = netdev_get_ifindex(ap->netdev);
+	int ret = -EINVAL;
-	int err = -EINVAL;
+	char **addr_str_list = NULL;
+	uint32_t static_addr = 0;
+	uint8_t prefix_len = 0;
+	char *gateway_str = NULL;
+	char **ip_range = NULL;
+	char **dns_str_list = NULL;
+	unsigned int lease_time = 0;
+	struct in_addr ia;
 
-	if (!l_settings_has_group(config, "IPv4"))
+	if (!l_settings_has_group(config, "IPv4") || !pool.used)
 		return 0;
 
-	err = ap_setup_dhcp(ap, config);
+	if (l_settings_has_key(config, "IPv4", "Address")) {
-	if (err == 0) {
+		addr_str_list = l_settings_get_string_list(config, "IPv4",
-		/* Address change required */
+								"Address", ',');
-		ap->rtnl_add_cmd = l_rtnl_ifaddr4_add(rtnl, ifindex,
+		if (!addr_str_list || !*addr_str_list) {
-					ap->ip_prefix, ap->own_ip,
+			l_error("Can't parse the profile [IPv4].Address "
-					broadcast_from_ip(ap->own_ip),
+				"setting as a string list");
-					ap_ifaddr4_added_cb, ap, NULL);
+			goto done;
-
-		if (!ap->rtnl_add_cmd) {
-			l_error("Failed to add IPv4 address");
-			return -EIO;
 		}
 
-		ap->cleanup_ip = true;
+		/* Check for the static IP syntax: Address=<IP> */
-
+		if (l_strv_length(addr_str_list) == 1 &&
-		*wait_dhcp = true;
+				inet_pton(AF_INET, *addr_str_list, &ia) == 1)
-		err = 0;
+			static_addr = ntohl(ia.s_addr);
-	/* Selected address already set, continue normally */
-	} else if (err == -EALREADY) {
-		*wait_dhcp = false;
-		err = 0;
 	}
 
-	return err;
+	if (l_settings_has_key(config, "IPv4", "Netmask")) {
+		L_AUTO_FREE_VAR(char *, netmask_str) =
+			l_settings_get_string(config, "IPv4", "Netmask");
+
+		if (inet_pton(AF_INET, netmask_str, &ia) != 1) {
+			l_error("Can't parse the profile [IPv4].Netmask "
+				"setting");
+			goto done;
+		}
+
+		prefix_len = __builtin_popcount(ia.s_addr);
+
+		if (ntohl(ia.s_addr) != util_netmask_from_prefix(prefix_len)) {
+			l_error("Invalid profile [IPv4].Netmask value");
+			goto done;
+		}
+	}
+
+	if (l_settings_has_key(config, "IPv4", "Gateway")) {
+		gateway_str = l_settings_get_string(config, "IPv4", "Gateway");
+		if (!gateway_str) {
+			l_error("Invalid profile [IPv4].Gateway value");
+			goto done;
+		}
+	}
+
+	if (l_settings_get_value(config, "IPv4", "IPRange")) {
+		int i;
+		uint32_t netmask;
+		uint8_t tmp_len = prefix_len ?: AP_DEFAULT_IPV4_PREFIX_LEN;
+
+		ip_range = l_settings_get_string_list(config, "IPv4",
+							"IPRange", ',');
+
+		if (!static_addr) {
+			l_error("[IPv4].IPRange only makes sense in an AP "
+				"profile if a static local address has also "
+				"been specified");
+			goto done;
+		}
+
+		if (!ip_range || l_strv_length(ip_range) != 2) {
+			l_error("Can't parse the profile [IPv4].IPRange "
+				"setting as two address strings");
+			goto done;
+		}
+
+		netmask = util_netmask_from_prefix(tmp_len);
+
+		for (i = 0; i < 2; i++) {
+			struct in_addr range_addr;
+
+			if (inet_aton(ip_range[i], &range_addr) != 1) {
+				l_error("Can't parse address in "
+					"[IPv4].IPRange[%i]", i + 1);
+				goto done;
+			}
+
+			if ((static_addr ^ ntohl(range_addr.s_addr)) &
+					netmask) {
+				ia.s_addr = htonl(static_addr);
+				l_error("[IPv4].IPRange[%i] is not in the "
+					"%s/%i subnet", i + 1, inet_ntoa(ia),
+					tmp_len);
+				goto done;
+			}
+		}
+	}
+
+	if (l_settings_has_key(config, "IPv4", "DNSList")) {
+		dns_str_list = l_settings_get_string_list(config, "IPv4",
+								"DNSList", ',');
+		if (!dns_str_list || !*dns_str_list) {
+			l_error("Can't parse the profile [IPv4].DNSList "
+				"setting as a string list");
+			goto done;
+		}
+	}
+
+	if (l_settings_has_key(config, "IPv4", "LeaseTime")) {
+		if (!l_settings_get_uint(config, "IPv4", "LeaseTime",
+						&lease_time) ||
+				lease_time < 1) {
+			l_error("Error parsing [IPv4].LeaseTime as a positive "
+				"integer");
+			goto done;
+		}
+	}
+
+	ret = ap_setup_netconfig4(ap, (const char **) addr_str_list, prefix_len,
+					gateway_str, (const char **) ip_range,
+					(const char **) dns_str_list,
+					lease_time);
+
+done:
+	l_strv_free(addr_str_list);
+	l_free(gateway_str);
+	l_strv_free(ip_range);
+	l_strv_free(dns_str_list);
+	return ret;
 }
 
 static bool ap_load_psk(struct ap_state *ap, const struct l_settings *config)
@@ -2722,7 +2835,7 @@ static bool ap_load_psk(struct ap_state *ap, const struct l_settings *config)
 }
 
 static int ap_load_config(struct ap_state *ap, const struct l_settings *config,
-				bool *out_wait_dhcp, bool *out_cck_rates)
+				bool *out_cck_rates)
 {
 	size_t len;
 	L_AUTO_FREE_VAR(char *, strval) = NULL;
@@ -2748,11 +2861,12 @@ static int ap_load_config(struct ap_state *ap, const struct l_settings *config,
 		return -EINVAL;
 
 	/*
-	 * This loads DHCP settings either from the l_settings object or uses
+	 * This looks at the network configuration settings in @config and
-	 * the defaults. wait_on_address will be set true if an address change
+	 * relevant global settings and if it determines that netconfig is to
-	 * is required.
+	 * be enabled for the AP, it both creates the DHCP server object and
+	 * processes IP settings, applying the defaults where needed.
 	 */
-	err = ap_load_dhcp(ap, config, out_wait_dhcp);
+	err = ap_load_ipv4(ap, config);
 	if (err)
 		return err;
 
@@ -2868,15 +2982,15 @@ struct ap_state *ap_start(struct netdev *netdev, struct l_settings *config,
 	struct ap_state *ap;
 	struct wiphy *wiphy = netdev_get_wiphy(netdev);
 	uint64_t wdev_id = netdev_get_wdev_id(netdev);
-	int err = -EINVAL;
+	int err;
-	bool wait_on_address = false;
 	bool cck_rates = true;
 
-	if (err_out)
+	if (L_WARN_ON(!config)) {
-		*err_out = err;
+		if (err_out)
+			*err_out = -EINVAL;
 
-	if (L_WARN_ON(!config))
 		return NULL;
+	}
 
 	ap = l_new(struct ap_state, 1);
 	ap->nl80211 = l_genl_family_new(iwd_get_genl(), NL80211_GENL_NAME);
@@ -2884,7 +2998,7 @@ struct ap_state *ap_start(struct netdev *netdev, struct l_settings *config,
 	ap->ops = ops;
 	ap->user_data = user_data;
 
-	err = ap_load_config(ap, config, &wait_on_address, &cck_rates);
+	err = ap_load_config(ap, config, &cck_rates);
 	if (err)
 		goto error;
 
@@ -2950,9 +3064,20 @@ struct ap_state *ap_start(struct netdev *netdev, struct l_settings *config,
 	if (!ap->mlme_watch)
 		l_error("Registering for MLME notification failed");
 
-	if (wait_on_address) {
+	if (ap->netconfig_set_addr4) {
-		if (err_out)
+		const char *broadcast_str =
-			*err_out = 0;
+			broadcast_from_ip(ap->netconfig_addr4_str);
+
+		ap->rtnl_add_cmd = l_rtnl_ifaddr4_add(rtnl,
+						netdev_get_ifindex(netdev),
+						ap->netconfig_prefix_len4,
+						ap->netconfig_addr4_str,
+						broadcast_str,
+						ap_ifaddr4_added_cb, ap, NULL);
+		if (!ap->rtnl_add_cmd) {
+			l_error("Failed to add the IPv4 address");
+			goto error;
+		}
 
 		return ap;
 	}