[PATCH 1/7] ap: Don't use L_AUTO_FREE_VAR with l_settings

[PATCH 1/7] ap: Don't use L_AUTO_FREE_VAR with l_settings

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 2145 bytes --]

L_AUTO_FREE_VAR only causes l_free to be called on the variable that is
freed and may leak the rest of the l_settings object's memory.
---
 src/ap.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index fd1a9a6b..f7b1cf9f 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -2544,8 +2544,8 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
 {
 	uint32_t ifindex = netdev_get_ifindex(ap->netdev);
 	char *passphrase;
-	L_AUTO_FREE_VAR(struct l_settings *, settings) = NULL;
-	int err;
+	struct l_settings *settings = NULL;
+	int err = -EINVAL;
 
 	/* No profile or DHCP settings */
 	if (!ap->config->profile && !pool.used)
@@ -2555,7 +2555,7 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
 		settings = l_settings_new();
 
 		if (!l_settings_load_from_file(settings, ap->config->profile))
-			return -EINVAL;
+			goto cleanup;
 
 		passphrase = l_settings_get_string(settings, "Security",
 							"Passphrase");
@@ -2564,7 +2564,7 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
 				l_error("[Security].Passphrase must not exceed "
 						"63 characters");
 				l_free(passphrase);
-				return -EINVAL;
+				goto cleanup;
 			}
 
 			strcpy(ap->config->passphrase, passphrase);
@@ -2573,7 +2573,8 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
 
 		if (!l_settings_has_group(settings, "IPv4")) {
 			*wait_dhcp = false;
-			return 0;
+			err = 0;
+			goto cleanup;
 		}
 	}
 
@@ -2587,21 +2588,22 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
 
 		if (!ap->rtnl_add_cmd) {
 			l_error("Failed to add IPv4 address");
-			return -EIO;
+			err = -EIO;
+			goto cleanup;
 		}
 
 		ap->cleanup_ip = true;
 
 		*wait_dhcp = true;
-
-		return 0;
+		err = 0;
 	/* Selected address already set, continue normally */
 	} else if (err == -EALREADY) {
 		*wait_dhcp = false;
-
-		return 0;
+		err = 0;
 	}
 
+cleanup:
+	l_settings_free(settings);
 	return err;
 }
 
-- 
2.27.0

[PATCH 2/7] ap: Fix an inet_aton error check

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 467 bytes --]

inet_aton return 0 on error, no a negative number.
---
 src/ap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ap.c b/src/ap.c
index f7b1cf9f..d9443097 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -213,7 +213,7 @@ static const char *broadcast_from_ip(const char *ip)
 	struct in_addr ia;
 	uint32_t bcast;
 
-	if (inet_aton(ip, &ia) < 0)
+	if (inet_aton(ip, &ia) != 1)
 		return NULL;
 
 	bcast = ntohl(ia.s_addr);
-- 
2.27.0

[PATCH 3/7] ap: Print error messages in dhcp_load_settings

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 2397 bytes --]

---
 src/ap.c | 48 ++++++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index d9443097..78df3355 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -2397,7 +2397,6 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
 static bool dhcp_load_settings(struct ap_state *ap, 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,
 							"IPv4", "Netmask");
@@ -2413,33 +2412,42 @@ static bool dhcp_load_settings(struct ap_state *ap, struct l_settings *settings)
 	if (!l_settings_get_uint(settings, "IPv4", "LeaseTime", &lease_time))
 		lease_time = 0;
 
-	if (ip_range && l_strv_length(ip_range) != 2)
-		goto parse_error;
-
-	if (netmask && !l_dhcp_server_set_netmask(server, netmask))
-		goto parse_error;
+	if (gateway && !l_dhcp_server_set_gateway(server, gateway)) {
+		l_error("[IPv4].Gateway value error");
+		goto error;
+	}
 
-	if (gateway && !l_dhcp_server_set_gateway(server, gateway))
-		goto parse_error;
+	if (dns && !l_dhcp_server_set_dns(server, dns)) {
+		l_error("[IPv4].DNSList value error");
+		goto error;
+	}
 
-	if (dns && !l_dhcp_server_set_dns(server, dns))
-		goto parse_error;
+	if (netmask && !l_dhcp_server_set_netmask(server, netmask)) {
+		l_error("[IPv4].Netmask value error");
+		goto error;
+	}
 
-	if (ip_range && !l_dhcp_server_set_ip_range(server, ip_range[0],
-							ip_range[1]))
-		goto parse_error;
+	if (ip_range) {
+		if (l_strv_length(ip_range) != 2) {
+			l_error("Two addresses expected in [IPv4].IPRange");
+			goto error;
+		}
 
-	if (lease_time && !l_dhcp_server_set_lease_time(server, lease_time))
-		goto parse_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 (netmask && inet_pton(AF_INET, netmask, &ia) > 0)
-		ap->ip_prefix = __builtin_popcountl(ia.s_addr);
-	else
-		ap->ip_prefix = 24;
+	if (lease_time && !l_dhcp_server_set_lease_time(server, lease_time)) {
+		l_error("[IPv4].LeaseTime value error");
+		goto error;
+	}
 
 	ret = true;
 
-parse_error:
+error:
 	l_strv_free(dns);
 	l_strv_free(ip_range);
 	return ret;
-- 
2.27.0

[PATCH 4/7] utils: Add util_netmask_from_prefix

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 549 bytes --]

---
 src/util.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/util.h b/src/util.h
index e6b4747f..d2fe0d75 100644
--- a/src/util.h
+++ b/src/util.h
@@ -110,4 +110,9 @@ static inline uint32_t util_secure_fill_with_msb(uint32_t val)
 bool util_ip_prefix_tohl(const char *ip, uint8_t *prefix, uint32_t *start_out,
 				uint32_t *end_out, uint32_t *mask_out);
 
+static inline uint32_t util_netmask_from_prefix(uint8_t prefix_len)
+{
+	return ~((1ull << (32 - prefix_len)) - 1);
+}
+
 #endif /* __UTIL_H */
-- 
2.27.0

[PATCH 5/7] ap: Refactor IP address selection

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 21536 bytes --]

Refactor how the subnet IP address is selected and change related
settings syntax in the following way:

* [IPv4].Address in an AP profile can now be given as just an IP address
  or a list of prefix notation address spaces.  In the latter case one
  subnet address of the size implied by [IPv4].Netmask is selected
  randomly from that space, defaults to a 28-bit netmask.  In the former
  case [IPv4].Address is now also checked for conflicts with addresses
  already in use.

* main.conf's [IPv4].APAddressPool is now the fallback setting that uses
  the same syntax as profile-local [IPv4].Address and has a default
  value of 192.168.0.0/16.

* main.conf's [General].APRanges is deprecated in favour of
  [IPv4].APAddressPool but this change should be backwards compatible.

ap_setup_dhcp now always prints error messages, it seems it was assumed
that the caller would do this before but it didn't.  It also now always
reads [IPv4].Netmask or the value already set on the interface (when
existing IP is used) and uses that value.  Previously the configured
netmask was only being announced through DHCP but not used locally.
---
 src/ap.c | 557 ++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 350 insertions(+), 207 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index 78df3355..98c89f7f 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -28,6 +28,7 @@
 #include <linux/if_ether.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
+#include <ifaddrs.h>
 
 #include <ell/ell.h>
 
@@ -81,12 +82,11 @@ struct ap_state {
 	struct l_dhcp_server *server;
 	uint32_t rtnl_add_cmd;
 	char *own_ip;
-	unsigned int ip_prefix;
+	uint8_t ip_prefix;
 
 	bool started : 1;
 	bool gtk_set : 1;
 	bool cleanup_ip : 1;
-	bool use_ip_pool : 1;
 };
 
 struct sta_state {
@@ -115,99 +115,10 @@ struct ap_wsc_pbc_probe_record {
 	uint64_t timestamp;
 };
 
-struct ap_ip_pool {
-	uint32_t start;
-	uint32_t end;
-	uint8_t prefix;
-
-	/* Fist/last valid subnet */
-	uint8_t sub_start;
-	uint8_t sub_end;
-
-	struct l_uintset *used;
-};
-
-struct ap_ip_pool pool;
+struct l_uintset *used_ips;
 static uint32_t netdev_watch;
 struct l_netlink *rtnl;
 
-/*
- * Creates pool of IPs which AP intefaces can use. Each call to ip_pool_get
- * will advance the subnet +1 so there are no IP conflicts between AP
- * interfaces
- */
-static bool ip_pool_create(const char *ip_prefix)
-{
-	if (!util_ip_prefix_tohl(ip_prefix, &pool.prefix, &pool.start,
-					&pool.end, NULL))
-		return false;
-
-	if (pool.prefix > 24) {
-		l_error("APRanges prefix must 24 or less (%u used)",
-				pool.prefix);
-		memset(&pool, 0, sizeof(pool));
-		return false;
-	}
-
-	/*
-	 * Find the number of subnets we can use, this will dictate the number
-	 * of AP interfaces that can be created (when using DHCP)
-	 */
-	pool.sub_start = (pool.start & 0x0000ff00) >> 8;
-	pool.sub_end = (pool.end & 0x0000ff00) >> 8;
-
-	pool.used = l_uintset_new_from_range(pool.sub_start, pool.sub_end);
-
-	return true;
-}
-
-static char *ip_pool_get()
-{
-	uint32_t ip;
-	struct in_addr ia;
-	uint8_t next_subnet = (uint8_t)l_uintset_find_unused_min(pool.used);
-
-	/* This shouldn't happen */
-	if (next_subnet < pool.sub_start || next_subnet > pool.sub_end)
-		return NULL;
-
-	l_uintset_put(pool.used, next_subnet);
-
-	ip = pool.start;
-	ip &= 0xffff00ff;
-	ip |= (next_subnet << 8);
-
-	ia.s_addr = htonl(ip);
-	return l_strdup(inet_ntoa(ia));
-}
-
-static bool ip_pool_put(const char *address)
-{
-	struct in_addr ia;
-	uint32_t ip;
-	uint8_t subnet;
-
-	if (inet_aton(address, &ia) < 0)
-		return false;
-
-	ip = ntohl(ia.s_addr);
-
-	subnet = (ip & 0x0000ff00) >> 8;
-
-	if (subnet < pool.sub_start || subnet > pool.sub_end)
-		return false;
-
-	return l_uintset_take(pool.used, subnet);
-}
-
-static void ip_pool_destroy()
-{
-	if (pool.used)
-		l_uintset_free(pool.used);
-
-	memset(&pool, 0, sizeof(pool));
-}
-
 static const char *broadcast_from_ip(const char *ip)
 {
 	struct in_addr ia;
@@ -326,13 +237,8 @@ static void ap_reset(struct ap_state *ap)
 					broadcast_from_ip(ap->own_ip),
 					NULL, NULL, NULL);
 
-	if (ap->own_ip) {
-		/* Release IP from pool if used */
-		if (ap->use_ip_pool)
-			ip_pool_put(ap->own_ip);
-
+	if (ap->own_ip)
 		l_free(ap->own_ip);
-	}
 
 	if (ap->server)
 		l_dhcp_server_stop(ap->server);
@@ -2394,12 +2300,12 @@ static void ap_mlme_notify(struct l_genl_msg *msg, void *user_data)
 	}
 }
 
-static bool dhcp_load_settings(struct ap_state *ap, struct l_settings *settings)
+static bool dhcp_load_settings(struct ap_state *ap,
+				const struct l_settings *settings,
+				uint32_t own_ip, uint8_t prefix)
 {
 	struct l_dhcp_server *server = ap->server;
 
-	L_AUTO_FREE_VAR(char *, netmask) = l_settings_get_string(settings,
-							"IPv4", "Netmask");
 	L_AUTO_FREE_VAR(char *, gateway) = l_settings_get_string(settings,
 							"IPv4", "Gateway");
 	char **dns = l_settings_get_string_list(settings, "IPv4",
@@ -2422,17 +2328,34 @@ static bool dhcp_load_settings(struct ap_state *ap, struct l_settings *settings)
 		goto error;
 	}
 
-	if (netmask && !l_dhcp_server_set_netmask(server, netmask)) {
-		l_error("[IPv4].Netmask value error");
-		goto error;
-	}
-
 	if (ip_range) {
+		int i;
+
 		if (l_strv_length(ip_range) != 2) {
 			l_error("Two addresses expected in [IPv4].IPRange");
 			goto error;
 		}
 
+		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 error;
+			}
+
+			if ((own_ip ^ ntohl(range_addr.s_addr)) &
+					util_netmask_from_prefix(prefix)) {
+				struct in_addr addr = { htonl(own_ip) };
+
+				l_error("[IPv4].IPRange[%i] is not in the "
+					"%s/%i subnet", i + 1,
+					inet_ntoa(addr), prefix);
+				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");
@@ -2453,31 +2376,234 @@ error:
 	return ret;
 }
 
+struct ap_ip_range {
+	uint32_t start;
+	uint32_t end;
+};
+
+/*
+ * Append any address ranges within an input start/end range, which contain
+ *@least one full subnet and don't intersect with any subnets already in
+ * use.  This may result in the input range being split into multiple ranges
+ * of different sizes or being skipped altogether.
+ * All inputs must be rounded to the subnet boundary and the @used queue
+ * sorted by the subnet start address.
+ */
+static void ap_append_range(struct l_queue *to, const struct ap_ip_range *range,
+				const struct l_queue *used)
+{
+	const struct l_queue_entry *entry =
+		l_queue_get_entries((struct l_queue *) used);
+	const struct ap_ip_range *used_range = entry ? entry->data : NULL;
+	uint32_t start = range->start;
+
+	while (range->end > start) {
+		while (used_range && used_range->end <= start) {
+			entry = entry->next;
+			used_range = entry ? entry->data : NULL;
+		}
+
+		/* No more used ranges that intersect with @start/@range->end */
+		if (!used_range || range->end <= used_range->start) {
+			struct ap_ip_range *sub = l_new(struct ap_ip_range, 1);
+
+			sub->start = start;
+			sub->end = range->end;
+			l_queue_push_tail(to, sub);
+			return;
+		}
+
+		/* Now we know @used_range is non-NULL and intersects */
+		if (start < used_range->start) {
+			struct ap_ip_range *sub = l_new(struct ap_ip_range, 1);
+
+			sub->start = start;
+			sub->end = used_range->start;
+			l_queue_push_tail(to, sub);
+		}
+
+		/* Skip to the start of the next subnet */
+		start = used_range->end;
+	}
+}
+
+static int ap_ip_range_compare(const void *a, const void *b, void *user_data)
+{
+	const struct ap_ip_range *range_a = a;
+	const struct ap_ip_range *range_b = b;
+
+	return range_a->start > range_b->start;
+}
+
+/* Select a subnet and a host address from a defined space */
+static int ap_select_addr(const char **addr_str_list, uint8_t prefix,
+				const char *ifname, uint32_t *out_addr)
+{
+	uint32_t total = 0;
+	uint32_t selected;
+	unsigned int i;
+	uint32_t addr;
+	struct ifaddrs *ifaddr;
+	const struct ifaddrs *ifa;
+	uint32_t subnet_size = 1 << (32 - prefix);
+	uint32_t host_mask = subnet_size - 1;
+	uint32_t subnet_mask = ~host_mask;
+	uint32_t host_addr = 0;
+	struct l_queue *ranges = l_queue_new();
+	struct l_queue *used = l_queue_new();
+	struct in_addr ia;
+	const struct l_queue_entry *entry;
+	int err = -EINVAL;
+
+	/* Build a sorted list of used/unavailable subnets */
+	if (getifaddrs(&ifaddr) != 0)
+		ifaddr = NULL;
+
+	for (ifa = ifaddr; ifa; ifa = ifa->ifa_next) {
+		const struct sockaddr_in *addr_sa;
+		const struct sockaddr_in *netmask_sa;
+		uint32_t used_addr;
+		uint32_t used_prefix;
+		struct ap_ip_range *range;
+
+		if (ifa->ifa_addr->sa_family != AF_INET)
+			continue;
+
+		/*
+		 * Ignore the subnet set on the target interface, we'll be
+		 * overwriting it if everything goes well.
+		 */
+		if (!strcmp(ifa->ifa_name, ifname))
+			continue;
+
+		addr_sa = (struct sockaddr_in *) ifa->ifa_addr;
+		netmask_sa = (struct sockaddr_in *) ifa->ifa_netmask;
+		if (!addr_sa || !netmask_sa)
+			continue;
+
+		used_addr = ntohl(addr_sa->sin_addr.s_addr);
+		used_prefix = __builtin_popcountl(netmask_sa->sin_addr.s_addr);
+
+		range = l_new(struct ap_ip_range, 1);
+		range->start = used_addr & subnet_mask;
+		range->end = (range->start + (1 << (32 - used_prefix)) +
+				subnet_size - 1) & subnet_mask;
+		l_queue_insert(used, range, ap_ip_range_compare, NULL);
+	}
+
+	freeifaddrs(ifaddr);
+
+	/* Build the list of available subnets */
+
+	/* Check for the static IP syntax: Address=<IP> */
+	if (l_strv_length((char **) addr_str_list) == 1 &&
+			inet_pton(AF_INET, *addr_str_list, &ia) == 1) {
+		struct ap_ip_range range;
+
+		host_addr = ntohl(ia.s_addr);
+		range.start = host_addr & subnet_mask;
+		range.end = range.start + subnet_size;
+		ap_append_range(ranges, &range, used);
+		goto select_addr;
+	}
+
+	for (i = 0; addr_str_list[i]; i++) {
+		struct ap_ip_range range;
+		uint8_t addr_prefix;
+
+		if (!util_ip_prefix_tohl(addr_str_list[i], &addr_prefix, &addr,
+						NULL, NULL)) {
+			l_error("Can't parse %s as a subnet address",
+				addr_str_list[i]);
+			goto done;
+		}
+
+		if (addr_prefix > prefix)
+			continue;
+
+		range.start = addr & subnet_mask;
+		range.end = range.start + (1 << (32 - addr_prefix));
+		ap_append_range(ranges, &range, used);
+	}
+
+select_addr:
+	if (l_queue_isempty(ranges)) {
+		l_error("No IP subnets available for new Access Point after "
+			"eliminating those already in use on the system");
+		err = -EEXIST;
+		goto done;
+	}
+
+	/* Count available @prefix-sized subnets */
+	for (entry = l_queue_get_entries(ranges); entry; entry = entry->next) {
+		struct ap_ip_range *range = entry->data;
+
+		total += (range->end - range->start) >> (32 - prefix);
+	}
+
+	selected = l_getrandom_uint32() % total;
+
+	/* Find the @selected'th @prefix-sized subnet */
+	for (entry = l_queue_get_entries(ranges);; entry = entry->next) {
+		struct ap_ip_range *range = entry->data;
+		uint32_t count = (range->end - range->start) >> (32 - prefix);
+
+		if (selected < count) {
+			addr = range->start + (selected << (32 - prefix));
+			break;
+		}
+
+		selected -= count;
+	}
+
+	if (!host_addr)
+		host_addr = addr + 1;
+
+	err = 0;
+	*out_addr = host_addr;
+
+done:
+	l_queue_destroy(ranges, l_free);
+	l_queue_destroy(used, l_free);
+	return err;
+}
+
+#define AP_DEFAULT_IPV4_PREFIX_LEN 28
+
 /*
- * This will determine the IP being used for DHCP. The IP will be automatically
- * set to ap->own_ip.
+ * This will determine the IP to be used for DHCP.  ap->server,
+ * ap->own_ip and ap->ip_prefix will be set.
  *
  * 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.
+ * 1. Select from address pool defined in the provisioning file
+ * 2. Keep address already set on the interface
+ * 3. Select from the global address pool
+ * 4. Select from 192.168.0.0/16
  *
  * Returns:  0 if an IP was successfully selected and needs to be set
  *          -EALREADY if an IP was already set on the interface
- *          -EEXIST if the IP pool ran out of IP's
+ *          -EEXIST if all available subnet addresses are in use
  *          -EINVAL if there was an error.
  */
-static int ap_setup_dhcp(struct ap_state *ap, struct l_settings *settings)
+static int ap_setup_dhcp(struct ap_state *ap,
+				const struct l_settings *profile,
+				const struct l_settings *global)
 {
 	uint32_t ifindex = netdev_get_ifindex(ap->netdev);
-	struct in_addr ia;
+	struct in_addr if_addr;
+	struct in_addr if_netmask;
 	uint32_t address = 0;
+	uint8_t prefix;
+	bool update = false;
 	int ret = -EINVAL;
+	char **addr_str_list;
+	L_AUTO_FREE_VAR(char *, netmask_str) =
+		l_settings_get_string(profile, "IPv4", "Netmask");
 
 	ap->server = l_dhcp_server_new(ifindex);
 	if (!ap->server) {
 		l_error("Failed to create DHCP server on %u", ifindex);
-		return -EINVAL;;
+		return -EINVAL;
 	}
 
 	if (getenv("IWD_DHCP_DEBUG"))
@@ -2485,87 +2611,137 @@ static int ap_setup_dhcp(struct ap_state *ap, struct l_settings *settings)
 							"[DHCPv4 SERV] ", NULL);
 
 	/* get the current address if there is one */
-	if (l_net_get_address(ifindex, &ia) && ia.s_addr != 0)
-		address = ia.s_addr;
+	if (l_net_get_address(ifindex, &if_addr) && if_addr.s_addr != 0 &&
+			l_net_get_netmask(ifindex, &if_netmask)) {
+		address = ntohl(if_addr.s_addr);
+		prefix = __builtin_popcountl(if_netmask.s_addr);
+	}
+
+	if (profile && l_settings_get_value(profile, "IPv4", "Address")) {
+		addr_str_list = l_settings_get_string_list(profile, "IPv4",
+								"Address", ',');
+		if (!addr_str_list || !*addr_str_list) {
+			l_error("Can't parse the profile [IPv4].Address "
+				"setting as a string list");
+			goto done;
+		}
+	} else if (address) {
+		addr_str_list = NULL;
+	} else if (l_settings_get_value(global, "IPv4", "APAddressPool")) {
+		addr_str_list = l_settings_get_string_list(global, "IPv4",
+							"APAddressPool", ',');
+		if (!addr_str_list || !*addr_str_list) {
+			l_error("Can't parse the global [IPv4].APAddressPool "
+				"setting as a string list");
+			goto done;
+		}
+	} else if (l_settings_get_value(global, "General", "APRanges")) {
+		addr_str_list = l_settings_get_string_list(global, "General",
+							"APRanges", ',');
+		if (!addr_str_list || !*addr_str_list) {
+			l_error("Can't parse the global [General].APRanges "
+				"setting as a string list");
+			goto done;
+		}
+	} else
+		addr_str_list = l_strv_append(NULL, "192.168.0.0/16");
 
-	if (ap->config->profile) {
-		char *addr;
-
-		addr = l_settings_get_string(settings, "IPv4", "Address");
-		if (addr) {
-			if (inet_pton(AF_INET, addr, &ia) < 0)
-				goto free_addr;
-
-			/* 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
-			goto free_addr;
+	if (addr_str_list) {
+		uint32_t new_addr;
+		int r;
 
-		/* Set the remaining DHCP options in config file */
-		if (!dhcp_load_settings(ap, settings)) {
-			ret = -EINVAL;
-			goto free_addr;
-		}
+		if (netmask_str) {
+			uint32_t netmask;
 
-		if (!l_dhcp_server_set_ip_address(ap->server, addr)) {
-			ret = -EINVAL;
-			goto free_addr;
-		}
+			if (inet_pton(AF_INET, netmask_str, &if_netmask) < 0) {
+				l_error("Can't parse the profile "
+					"[IPv4].Netmask setting");
+				goto done;
+			}
 
-		ap->own_ip = l_strdup(addr);
+			netmask = if_netmask.s_addr;
+			prefix = __builtin_popcountl(netmask);
+			if ((ntohl(netmask) !=
+					util_netmask_from_prefix(prefix))) {
+				l_error("Invalid netmask %s", netmask_str);
+				goto done;
+			}
 
-free_addr:
-		l_free(addr);
+			if (prefix > 30 || prefix < 8) {
+				l_error("Netmasks between 8 and 30 bits "
+					"are allowed");
+				goto done;
+			}
+		} else
+			prefix = AP_DEFAULT_IPV4_PREFIX_LEN;
 
-		return ret;
-	} else if (address) {
-		/* No config file and address is already set */
-		ap->own_ip = l_strdup(inet_ntoa(ia));
-
-		return -EALREADY;
-	} else if (pool.used) {
-		/* No config file, no address set. Use IP pool */
-		ap->own_ip = ip_pool_get();
-		if (!ap->own_ip) {
-			l_error("No more IP's in pool, cannot start AP on %u",
-					ifindex);
-			return -EEXIST;
+		r = ap_select_addr((const char **) addr_str_list, prefix,
+					netdev_get_name(ap->netdev), &new_addr);
+		if (r < 0) {
+			ret = r;
+			goto done;
 		}
 
-		ap->use_ip_pool = true;
-		ap->ip_prefix = pool.prefix;
+		if (new_addr != address) {
+			address = new_addr;
+			update = true;
+		}
+	}
 
-		return 0;
+	if_addr.s_addr = htonl(address);
+	if (!l_dhcp_server_set_ip_address(ap->server, inet_ntoa(if_addr))) {
+		l_error("l_dhcp_server_set_ip_address failed");
+		goto done;
+	}
+
+	if_netmask.s_addr = htonl(util_netmask_from_prefix(prefix));
+	if (!l_dhcp_server_set_netmask(ap->server, inet_ntoa(if_netmask))) {
+		l_error("l_dhcp_server_set_netmask failed");
+		goto done;
 	}
 
-	return -EINVAL;
+	/* Set the remaining DHCP options in config file */
+	if (profile && !dhcp_load_settings(ap, profile, address, prefix))
+		goto done;
+
+	ap->own_ip = l_strdup(inet_ntoa(if_addr));
+	ap->ip_prefix = prefix;
+	ret = update ? 0 : -EALREADY;
+
+done:
+	l_strv_free(addr_str_list);
+	return ret;
 }
 
 static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
 {
 	uint32_t ifindex = netdev_get_ifindex(ap->netdev);
 	char *passphrase;
-	struct l_settings *settings = NULL;
+	struct l_settings *profile = NULL;
 	int err = -EINVAL;
+	const struct l_settings *settings = iwd_get_config();
+	bool dhcp_enable;
 
-	/* No profile or DHCP settings */
-	if (!ap->config->profile && !pool.used)
+	/*
+	 * Enable DHCP server only if [General].EnableNetworkConfiguration is
+	 * true.  With StartProfile, individual profiles can additionally
+	 * disable DHCP by not having the [IPv4] group.
+	 */
+	if (!l_settings_get_bool(settings, "General",
+					"EnableNetworkConfiguration",
+					&dhcp_enable) ||
+			!dhcp_enable) {
+		*wait_dhcp = false;
 		return 0;
+	}
 
 	if (ap->config->profile) {
-		settings = l_settings_new();
+		profile = l_settings_new();
 
-		if (!l_settings_load_from_file(settings, ap->config->profile))
+		if (!l_settings_load_from_file(profile, ap->config->profile))
 			goto cleanup;
 
-		passphrase = l_settings_get_string(settings, "Security",
+		passphrase = l_settings_get_string(profile, "Security",
 							"Passphrase");
 		if (passphrase) {
 			if (strlen(passphrase) > 63) {
@@ -2579,14 +2755,14 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
 			l_free(passphrase);
 		}
 
-		if (!l_settings_has_group(settings, "IPv4")) {
+		if (!l_settings_has_group(profile, "IPv4")) {
 			*wait_dhcp = false;
 			err = 0;
 			goto cleanup;
 		}
 	}
 
-	err = ap_setup_dhcp(ap, settings);
+	err = ap_setup_dhcp(ap, profile, settings);
 	if (err == 0) {
 		/* Address change required */
 		ap->rtnl_add_cmd = l_rtnl_ifaddr4_add(rtnl, ifindex,
@@ -2611,7 +2787,7 @@ static int ap_load_profile_and_dhcp(struct ap_state *ap, bool *wait_dhcp)
 	}
 
 cleanup:
-	l_settings_free(settings);
+	l_settings_free(profile);
 	return err;
 }
 
@@ -3309,9 +3485,6 @@ static void ap_netdev_watch(struct netdev *netdev,
 
 static int ap_init(void)
 {
-	const struct l_settings *settings = iwd_get_config();
-	bool dhcp_enable;
-
 	netdev_watch = netdev_watch_add(ap_netdev_watch, NULL, NULL);
 
 	l_dbus_register_interface(dbus_get_bus(), IWD_AP_INTERFACE,
@@ -3320,34 +3493,6 @@ static int ap_init(void)
 			ap_setup_diagnostic_interface,
 			ap_diagnostic_interface_destroy, false);
 
-	/*
-	 * Reusing [General].EnableNetworkConfiguration as a switch to enable
-	 * DHCP server. If no value is found or it is false do not create a
-	 * DHCP server.
-	 */
-	if (!l_settings_get_bool(settings, "General",
-				"EnableNetworkConfiguration", &dhcp_enable))
-		dhcp_enable = false;
-
-	if (dhcp_enable) {
-		L_AUTO_FREE_VAR(char *, ip_prefix);
-
-		ip_prefix = l_settings_get_string(settings, "General",
-							"APRanges");
-		/*
-		 * In this case its assumed the user only cares about station
-		 * netconfig so we let ap_init pass but DHCP will not be
-		 * enabled.
-		 */
-		if (!ip_prefix) {
-			l_warn("[General].APRanges must be set for DHCP");
-			return 0;
-		}
-
-		if (!ip_pool_create(ip_prefix))
-			return -EINVAL;
-	}
-
 	rtnl = iwd_get_rtnl();
 
 	return 0;
@@ -3357,8 +3502,6 @@ static void ap_exit(void)
 {
 	netdev_watch_remove(netdev_watch);
 	l_dbus_unregister_interface(dbus_get_bus(), IWD_AP_INTERFACE);
-
-	ip_pool_destroy();
 }
 
 IWD_MODULE(ap, ap_init, ap_exit)
-- 
2.27.0

[PATCH 6/7] autotests: Update APRanges usage in testAP

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 717 bytes --]

[General].APRange is now [IPv4].APAddressPool and the netmask is changed
from 23 to 27 bits to make the test correctly assert that only two
default-sized subnets are allowed by IWD simultaneously (default has
changed from 24 to 28 bits)
---
 autotests/testAP/dhcp/main.conf | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/autotests/testAP/dhcp/main.conf b/autotests/testAP/dhcp/main.conf
index 667de23c..62ae782d 100644
--- a/autotests/testAP/dhcp/main.conf
+++ b/autotests/testAP/dhcp/main.conf
@@ -3,4 +3,6 @@ DisableMacAddressRandomization=true
 
 [General]
 EnableNetworkConfiguration=true
-APRanges=192.168.80.0/23
+
+[IPv4]
+APAddressPool=192.168.80.0/27
-- 
2.27.0

[PATCH 7/7] doc: Update AP settings in iwd.ap and iwd.config(5)

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 8135 bytes --]

Update the AP-mode address logic documentation, change some of the
wording and add some references.
---
 src/iwd.ap.rst     | 70 +++++++++++++++++++++++++++++-----------------
 src/iwd.config.rst | 30 ++++++++++++--------
 2 files changed, 63 insertions(+), 37 deletions(-)

diff --git a/src/iwd.ap.rst b/src/iwd.ap.rst
index 52312855..57081702 100644
--- a/src/iwd.ap.rst
+++ b/src/iwd.ap.rst
@@ -3,7 +3,7 @@
 ============
 
 --------------------------------------
-Configuration of IWD access point
+Configuration of IWD access points
 --------------------------------------
 
 :Author: James Prestwood <prestwoj@gmail.com>
@@ -25,19 +25,21 @@ Description of access point provisioning files.
 DESCRIPTION
 ===========
 
-An access point provisioning files define the configuration of an IWD access
+An access point provisioning file defines the configuration of an IWD access
 point. These files live in *$STATE_DIRECTORY*/ap (/var/lib/iwd/ap by default).
+They are read when the `net.connman.iwd.AccessPoint.StartProfile(ssid)` DBus
+method is used.
 
 FILE FORMAT
 ===========
 
-See *iwd.network* for details on the file format.
+See *iwd.network* for details on the settings file syntax.
 
 SETTINGS
 ========
 
 The settings are split into several categories.  Each category has a group
-associated with it and described in separate tables below.
+associated with it and is described in the corresponding table below.
 
 Network Authentication Settings
 -------------------------------
@@ -54,13 +56,15 @@ configuration.
    * - Passphrase
      - 8..63 character string
 
-       Passphrase to be used with this access point.
+       PSK Passphrase to be used with this access point.
 
-DHCP Server Settings
---------------------
+IPv4 Network Configuration
+--------------------------
 
-The group ``[IPv4]`` contains settings for IWD's built in DHCP server. All
-settings are optional.
+The group ``[IPv4]`` contains settings for IWD's built-in DHCP server. All
+settings are optional.  They're used if network configuration was enabled as
+described in ``iwd.config(5)``.  Omitting the ``[IPv4]`` group disables
+network configuration and the DHCP server for this access point.
 
 .. list-table::
    :header-rows: 0
@@ -68,41 +72,57 @@ settings are optional.
    :widths: 20 80
 
    * - Address
-     - IP Address of AP
-
-       Optional address for the DHCP server/access point. If provided this
-       address will be set on the AP interface and any other DHCP server options
-       will be derived from this address, unless they are overriden inside the
-       AP profile. If [IPv4].Address is not provided and no IP address is set
-       on the interface prior to calling StartProfile the IP pool will be used.
+     - Local IP address of the AP or a comma-separated list of prefix-notation
+       addresses
+
+       Optional local address for the access point and the DHCP server.  If a
+       single address is provided this address will be set on the AP interface
+       and any other DHCP server options will be derived from it, unless they
+       are overriden by other settings.  If a list of addresses and prefix
+       lengths is given (in the `<IP>/<prefix-len>` format), a single subnet
+       address will be selected from the available space each time this profile
+       is started.  The subnet size will be based on the ``[IPv4].Netmask``
+       setting.  If ``[IPv4].Address`` is not provided and no IP address is set
+       on the interface prior to calling `StartProfile` the IP pool defined by
+       the global ``[IPv4].APAddressPool`` setting will be used, which in turn
+       defaults to 192.168.0.0/16.
+       For example, if ``[IPv4].Netmask`` is set the 255.255.255.0 and this
+       setting, or the global ``APAddressPool`` fallback, is set to
+       ``192.168.0.0/16, 10.0.0.0/22``, IWD will select one of the 256 subnets
+       with addresses in the 192.168.<0-255>.0/24 range or one of the 4 subnets
+       with addresses in the 10.0.<0-3>.0/24 range, allowing 270 possible
+       subnets.  Defining an address pool larger than the subnet may be
+       desired if other interfaces on the system use dynamically assigned
+       addresses giving IWD a chance to avoid conflicts.
 
    * - Gateway
      - IP Address of gateway
 
-       IP address of gateway. This will inherit from [IPv4].Address if not
-       provided.
+       IP address of the gateway to be advertised by DHCP. This will fall back
+       to the local IP address if not provided.
 
    * - Netmask
-     - Netmask of DHCP server
+     - Local netmask of the AP
 
-       This will be generated from [IPv4].Address if not provided.
+       Defaults to a 28-bit netmask if not provided.
 
    * - DNSList
-     - List of DNS servers
+     - List of DNS servers as a comma-separated IP address list
 
        A list of DNS servers which will be advertised by the DHCP server. If
        not provided no DNS servers will be sent by the DHCP server.
 
    * - LeaseTime
-     - Time limit for DHCP leases
+     - Time limit for DHCP leases in seconds
 
        Override the default lease time.
 
    * - IPRange
-     - Range of IPs to use for the DHCP server
+     - Range of IPs for the DHCP server to use given as two addresses
+       separated by a comma
 
-       If not provided a default range will be chosen which is the DHCP server
-       address + 1 to 254.
+       From and to addresses of the range assigned to clients by DHCP.  If not
+       provided the range from local address + 1 to .254 will be used.
 
 SEE ALSO
 ========
diff --git a/src/iwd.config.rst b/src/iwd.config.rst
index 478bd616..b694bc8d 100644
--- a/src/iwd.config.rst
+++ b/src/iwd.config.rst
@@ -67,23 +67,13 @@ The group ``[General]`` contains general settings.
        obtain the dynamic addresses from the network through the built-in
        DHCP client.
 
-       This also enables DHCP server when in AP mode when either
-       [General].APRanges is set or an AP profile is being used.
+       This also enables network configuration and the DHCP server when in AP
+       mode and the AP profile being activated does does not override it.
 
        The network configuration feature is disabled by default.  See
        ``[Network]`` settings for additional settings related to network
        configuration.
 
-   * - APRanges
-     - Values: <IP in prefix notation>
-
-       Sets the range of IP's used for DHCP server (AP mode). The IP should be
-       in prefix notation e.g. 192.168.1.0/24. AP's which are started in a
-       profile-less configuration will use this pool of IP's to set the AP's
-       interface address as well as default DHCP server options. Each AP will
-       get a new subnet from the range and clients will be addressed in that
-       subnet to avoid IP conflicts if multiple AP's are started.
-
    * - UseDefaultInterface
      - Values: true, **false**
 
@@ -312,6 +302,22 @@ No modification from defaults is normally required.
        prevent **iwd** from roaming properly, but can be useful for networks
        operating under extremely low rssi levels where roaming isn't possible.
 
+IPv4
+----
+
+The group ``[IPv4]`` contains settings related to IPv4 address configuration.
+
+   * - APAddressPool
+     - Values: comma-separated list of prefix-notation IPv4 address strings
+
+       Defines the space of IPs used for the AP-mode subnet addresses and the
+       DHCP server.  Defaults to 192.168.0.0/16.  The prefix length defines
+       the space from which an address is selected but the actual subnet size
+       (netmask) is based on the AP profile being activated and defaults to 28
+       bits.  The AP profile's [IPv4].Address setting overrides the global
+       value here.  Setting a too small address space will limit the number of
+       access points that can be running simultaneously on different interfaces.
+
 SEE ALSO
 ========
 
-- 
2.27.0

[PATCH 3/7] ap: Print error messages in dhcp_load_settings

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 2115 bytes --]

---
 src/ap.c | 42 ++++++++++++++++++++++++++++--------------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/src/ap.c b/src/ap.c
index d9443097..2042cde6 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -2413,24 +2413,38 @@ static bool dhcp_load_settings(struct ap_state *ap, struct l_settings *settings)
 	if (!l_settings_get_uint(settings, "IPv4", "LeaseTime", &lease_time))
 		lease_time = 0;
 
-	if (ip_range && l_strv_length(ip_range) != 2)
-		goto parse_error;
+	if (gateway && !l_dhcp_server_set_gateway(server, gateway)) {
+		l_error("[IPv4].Gateway value error");
+		goto error;
+	}
 
-	if (netmask && !l_dhcp_server_set_netmask(server, netmask))
-		goto parse_error;
+	if (dns && !l_dhcp_server_set_dns(server, dns)) {
+		l_error("[IPv4].DNSList value error");
+		goto error;
+	}
 
-	if (gateway && !l_dhcp_server_set_gateway(server, gateway))
-		goto parse_error;
+	if (netmask && !l_dhcp_server_set_netmask(server, netmask)) {
+		l_error("[IPv4].Netmask value error");
+		goto error;
+	}
 
-	if (dns && !l_dhcp_server_set_dns(server, dns))
-		goto parse_error;
+	if (ip_range) {
+		if (l_strv_length(ip_range) != 2) {
+			l_error("Two addresses expected in [IPv4].IPRange");
+			goto error;
+		}
 
-	if (ip_range && !l_dhcp_server_set_ip_range(server, ip_range[0],
-							ip_range[1]))
-		goto parse_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))
-		goto parse_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);
@@ -2439,7 +2453,7 @@ static bool dhcp_load_settings(struct ap_state *ap, struct l_settings *settings)
 
 	ret = true;
 
-parse_error:
+error:
 	l_strv_free(dns);
 	l_strv_free(ip_range);
 	return ret;
-- 
2.27.0