inet: Do not add broadcast address for P2P/VPNs

Adding broadcast address for VPNs is not permitted by "ip route" and it
messes up the use of getifaddrs() for getting the destination address.
Adding broadcast address for P2P connections is also unnecessary. This
change adds a simple boolean to toggle whether the address change
concerns a VPN or any other service, to toggle adding of IFA_BROADCAST.
Naturally this only affects IPv4 VPNs.

The issue fixed here is easy to reprocude by connecting a VPN, then
checking the destination address with ioctl() using SIOCGIFDSTADDR flag for
IFF_POINTOPOINT type interface, and then using getifaddrs() for the same
interface by checking the ifa_flags for flag IFF_POINTOPOINT to get the
destination address.

The toggle for P2P/VPN connections is saved in struct connman_ipaddress,
and is set with new function connman_ipaddress_set_p2p() to avoid
API/ABI breakage.

Also changed connman_inet_clear_ipv6_address() to use the struct
connman_ipaddress as other set/clear_ipv4/6_address functions do.

20 files changed, 123 insertions, 36 deletions

diff --git a/include/inet.h b/include/inet.h
index c76e487d..9245eef8 100644
--- a/include/inet.h
+++ b/include/inet.h
@@ -55,7 +55,7 @@ bool connman_inet_compare_ipv6_subnet(int index, const char *host);
 int connman_inet_set_ipv6_address(int index,
 		struct connman_ipaddress *ipaddress);
 int connman_inet_clear_ipv6_address(int index,
-		const char *address, int prefix_len);
+					struct connman_ipaddress *ipaddress);
 int connman_inet_add_ipv6_network_route(int index, const char *host,
 					const char *gateway, unsigned char prefix_len);
 int connman_inet_add_ipv6_host_route(int index, const char *host,
diff --git a/include/ipaddress.h b/include/ipaddress.h
index 3655ca86..652db0f0 100644
--- a/include/ipaddress.h
+++ b/include/ipaddress.h
@@ -22,6 +22,8 @@
 #ifndef __CONNMAN_IPADDRESS_H
 #define __CONNMAN_IPADDRESS_H
 
+#include <stdbool.h>
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -36,6 +38,8 @@ struct connman_ipaddress;
 
 unsigned char connman_ipaddress_calc_netmask_len(const char *netmask);
 struct connman_ipaddress *connman_ipaddress_alloc(int family);
+void connman_ipaddress_set_p2p(struct connman_ipaddress *ipaddress,
+				bool value);
 void connman_ipaddress_free(struct connman_ipaddress *ipaddress);
 int connman_ipaddress_set_ipv4(struct connman_ipaddress *ipaddress,
 				const char *address, const char *netmask,
diff --git a/plugins/dundee.c b/plugins/dundee.c
index b5420acf..57571ec3 100644
--- a/plugins/dundee.c
+++ b/plugins/dundee.c
@@ -488,7 +488,7 @@ static void extract_settings(DBusMessageIter *array,
 	if (index < 0)
 		goto out;
 
-	info->address = connman_ipaddress_alloc(CONNMAN_IPCONFIG_TYPE_IPV4);
+	info->address = connman_ipaddress_alloc(AF_INET);
 	if (!info->address)
 		goto out;
 
diff --git a/plugins/ofono.c b/plugins/ofono.c
index 36873d5a..f0bd3c5d 100644
--- a/plugins/ofono.c
+++ b/plugins/ofono.c
@@ -910,7 +910,7 @@ static void extract_ipv4_settings(DBusMessageIter *array,
 	if (context->ipv4_method != CONNMAN_IPCONFIG_METHOD_FIXED)
 		goto out;
 
-	context->ipv4_address = connman_ipaddress_alloc(CONNMAN_IPCONFIG_TYPE_IPV4);
+	context->ipv4_address = connman_ipaddress_alloc(AF_INET);
 	if (!context->ipv4_address) {
 		context->index = -1;
 		goto out;
@@ -998,7 +998,7 @@ static void extract_ipv6_settings(DBusMessageIter *array,
 	context->ipv6_method = CONNMAN_IPCONFIG_METHOD_AUTO;
 
 	context->ipv6_address =
-		connman_ipaddress_alloc(CONNMAN_IPCONFIG_TYPE_IPV6);
+		connman_ipaddress_alloc(AF_INET6);
 	if (!context->ipv6_address)
 		goto out;
 
diff --git a/plugins/vpn.c b/plugins/vpn.c
index 7bb69297..95994c79 100644
--- a/plugins/vpn.c
+++ b/plugins/vpn.c
@@ -437,6 +437,7 @@ static int extract_ip(DBusMessageIter *array, int family,
 	}
 
 	connman_ipaddress_set_peer(data->ip, peer);
+	connman_ipaddress_set_p2p(data->ip, true);
 
 	return 0;
 }
diff --git a/src/bridge.c b/src/bridge.c
index cd2d9cee..df19a6af 100644
--- a/src/bridge.c
+++ b/src/bridge.c
@@ -122,7 +122,8 @@ int __connman_bridge_enable(const char *name, const char *ip_address,
 
 	err = __connman_inet_modify_address(RTM_NEWADDR,
 				NLM_F_REPLACE | NLM_F_ACK, index, AF_INET,
-				ip_address, NULL, prefix_len, broadcast);
+				ip_address, NULL, prefix_len, broadcast,
+				false);
 	if (err < 0)
 		return err;
 
diff --git a/src/connman.h b/src/connman.h
index 560ef46c..c6d814dc 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -159,7 +159,8 @@ int __connman_inet_modify_address(int cmd, int flags, int index, int family,
 				const char *address,
 				const char *peer,
 				unsigned char prefixlen,
-				const char *broadcast);
+				const char *broadcast,
+				bool is_p2p);
 int __connman_inet_get_interface_address(int index, int family, void *address);
 int __connman_inet_get_interface_ll_address(int index, int family, void *address);
 int __connman_inet_get_interface_mac_address(int index, uint8_t *mac_address);
@@ -302,6 +303,7 @@ struct connman_ipaddress {
 	char *peer;
 	char *broadcast;
 	char *gateway;
+	bool is_p2p; /* P2P connection or VPN, broadcast is excluded. */
 };
 
 struct connman_ipconfig_ops {
diff --git a/src/inet.c b/src/inet.c
index 8cbc3e46..2d30c3b6 100644
--- a/src/inet.c
+++ b/src/inet.c
@@ -79,7 +79,8 @@ int __connman_inet_modify_address(int cmd, int flags,
 				const char *address,
 				const char *peer,
 				unsigned char prefixlen,
-				const char *broadcast)
+				const char *broadcast,
+				bool is_p2p)
 {
 	uint8_t request[NLMSG_ALIGN(sizeof(struct nlmsghdr)) +
 			NLMSG_ALIGN(sizeof(struct ifaddrmsg)) +
@@ -94,8 +95,9 @@ int __connman_inet_modify_address(int cmd, int flags,
 	int sk, err;
 
 	DBG("cmd %#x flags %#x index %d family %d address %s peer %s "
-		"prefixlen %hhu broadcast %s", cmd, flags, index, family,
-		address, peer, prefixlen, broadcast);
+		"prefixlen %hhu broadcast %s p2p %s", cmd, flags, index,
+		family, address, peer, prefixlen, broadcast,
+		is_p2p ? "true" : "false");
 
 	if (!address)
 		return -EINVAL;
@@ -122,12 +124,6 @@ int __connman_inet_modify_address(int cmd, int flags,
 		if (inet_pton(AF_INET, address, &ipv4_addr) < 1)
 			return -1;
 
-		if (broadcast)
-			inet_pton(AF_INET, broadcast, &ipv4_bcast);
-		else
-			ipv4_bcast.s_addr = ipv4_addr.s_addr |
-				htonl(0xfffffffflu >> prefixlen);
-
 		if (peer) {
 			if (inet_pton(AF_INET, peer, &ipv4_dest) < 1)
 				return -1;
@@ -149,14 +145,25 @@ int __connman_inet_modify_address(int cmd, int flags,
 		if (err < 0)
 			return err;
 
-		err = __connman_inet_rtnl_addattr_l(header,
-						sizeof(request),
-						IFA_BROADCAST,
-						&ipv4_bcast,
-						sizeof(ipv4_bcast));
-		if (err < 0)
-			return err;
+		/*
+		 * Broadcast address must not be added for P2P / VPN as
+		 * getifaddrs() cannot interpret destination address.
+		 */
+		if (!is_p2p) {
+			if (broadcast)
+				inet_pton(AF_INET, broadcast, &ipv4_bcast);
+			else
+				ipv4_bcast.s_addr = ipv4_addr.s_addr |
+					htonl(0xfffffffflu >> prefixlen);
 
+			err = __connman_inet_rtnl_addattr_l(header,
+							sizeof(request),
+							IFA_BROADCAST,
+							&ipv4_bcast,
+							sizeof(ipv4_bcast));
+			if (err < 0)
+				return err;
+		}
 	} else if (family == AF_INET6) {
 		if (inet_pton(AF_INET6, address, &ipv6_addr) < 1)
 			return -1;
@@ -435,18 +442,20 @@ int connman_inet_set_ipv6_address(int index,
 	int err;
 	unsigned char prefix_len;
 	const char *address;
+	bool is_p2p;
 
 	if (!ipaddress->local)
 		return 0;
 
 	prefix_len = ipaddress->prefixlen;
 	address = ipaddress->local;
+	is_p2p = ipaddress->is_p2p;
 
 	DBG("index %d address %s prefix_len %d", index, address, prefix_len);
 
 	err = __connman_inet_modify_address(RTM_NEWADDR,
 				NLM_F_REPLACE | NLM_F_ACK, index, AF_INET6,
-				address, NULL, prefix_len, NULL);
+				address, NULL, prefix_len, NULL, is_p2p);
 	if (err < 0) {
 		connman_error("%s: %s", __func__, strerror(-err));
 		return err;
@@ -460,6 +469,7 @@ int connman_inet_set_address(int index, struct connman_ipaddress *ipaddress)
 	int err;
 	unsigned char prefix_len;
 	const char *address, *broadcast, *peer;
+	bool is_p2p;
 
 	if (!ipaddress->local)
 		return -1;
@@ -468,12 +478,13 @@ int connman_inet_set_address(int index, struct connman_ipaddress *ipaddress)
 	address = ipaddress->local;
 	broadcast = ipaddress->broadcast;
 	peer = ipaddress->peer;
+	is_p2p = ipaddress->is_p2p;
 
 	DBG("index %d address %s prefix_len %d", index, address, prefix_len);
 
 	err = __connman_inet_modify_address(RTM_NEWADDR,
 				NLM_F_REPLACE | NLM_F_ACK, index, AF_INET,
-				address, peer, prefix_len, broadcast);
+				address, peer, prefix_len, broadcast, is_p2p);
 	if (err < 0) {
 		connman_error("%s: %s", __func__, strerror(-err));
 		return err;
@@ -482,10 +493,17 @@ int connman_inet_set_address(int index, struct connman_ipaddress *ipaddress)
 	return 0;
 }
 
-int connman_inet_clear_ipv6_address(int index, const char *address,
-							int prefix_len)
+int connman_inet_clear_ipv6_address(int index,
+					struct connman_ipaddress *ipaddress)
 {
 	int err;
+	int prefix_len;
+	const char *address;
+	bool is_p2p;
+
+	address = ipaddress->local;
+	prefix_len = ipaddress->prefixlen;
+	is_p2p = ipaddress->is_p2p;
 
 	DBG("index %d address %s prefix_len %d", index, address, prefix_len);
 
@@ -493,7 +511,7 @@ int connman_inet_clear_ipv6_address(int index, const char *address,
 		return -EINVAL;
 
 	err = __connman_inet_modify_address(RTM_DELADDR, 0, index, AF_INET6,
-				address, NULL, prefix_len, NULL);
+				address, NULL, prefix_len, NULL, is_p2p);
 	if (err < 0) {
 		connman_error("%s: %s", __func__, strerror(-err));
 		return err;
@@ -507,11 +525,13 @@ int connman_inet_clear_address(int index, struct connman_ipaddress *ipaddress)
 	int err;
 	unsigned char prefix_len;
 	const char *address, *broadcast, *peer;
+	bool is_p2p;
 
 	prefix_len = ipaddress->prefixlen;
 	address = ipaddress->local;
 	broadcast = ipaddress->broadcast;
 	peer = ipaddress->peer;
+	is_p2p = ipaddress->is_p2p;
 
 	DBG("index %d address %s prefix_len %d peer %s broadcast %s", index,
 		address, prefix_len, peer, broadcast);
@@ -520,7 +540,7 @@ int connman_inet_clear_address(int index, struct connman_ipaddress *ipaddress)
 		return -EINVAL;
 
 	err = __connman_inet_modify_address(RTM_DELADDR, 0, index, AF_INET,
-				address, peer, prefix_len, broadcast);
+				address, peer, prefix_len, broadcast, is_p2p);
 	if (err < 0) {
 		connman_error("%s: %s", __func__, strerror(-err));
 		return err;
diff --git a/src/ipaddress.c b/src/ipaddress.c
index d63d95c3..f728eab3 100644
--- a/src/ipaddress.c
+++ b/src/ipaddress.c
@@ -70,10 +70,19 @@ struct connman_ipaddress *connman_ipaddress_alloc(int family)
 	ipaddress->peer = NULL;
 	ipaddress->broadcast = NULL;
 	ipaddress->gateway = NULL;
+	ipaddress->is_p2p = false;
 
 	return ipaddress;
 }
 
+void connman_ipaddress_set_p2p(struct connman_ipaddress *ipaddress, bool value)
+{
+	if (!ipaddress)
+		return;
+
+	ipaddress->is_p2p = value;
+}
+
 void connman_ipaddress_free(struct connman_ipaddress *ipaddress)
 {
 	if (!ipaddress)
@@ -223,6 +232,7 @@ connman_ipaddress_copy(struct connman_ipaddress *ipaddress)
 	copy->peer = g_strdup(ipaddress->peer);
 	copy->broadcast = g_strdup(ipaddress->broadcast);
 	copy->gateway = g_strdup(ipaddress->gateway);
+	copy->is_p2p = ipaddress->is_p2p;
 
 	return copy;
 }
diff --git a/src/ipconfig.c b/src/ipconfig.c
index a1073a75..2bfeefd4 100644
--- a/src/ipconfig.c
+++ b/src/ipconfig.c
@@ -696,6 +696,25 @@ static inline gint check_duplicate_address(gconstpointer a, gconstpointer b)
 	return g_strcmp0(addr1->local, addr2->local);
 }
 
+static bool is_index_p2p_service(int index)
+{
+	struct connman_service *service;
+	enum connman_service_type type;
+
+	service = __connman_service_lookup_from_index(index);
+	if (!service)
+		return false;
+
+	type = connman_service_get_type(service);
+	switch (type) {
+	case CONNMAN_SERVICE_TYPE_P2P:
+	case CONNMAN_SERVICE_TYPE_VPN:
+		return true;
+	default:
+		return false;
+	}
+}
+
 int __connman_ipconfig_newaddr(int index, int family, const char *label,
 				unsigned char prefixlen, const char *address)
 {
@@ -718,6 +737,9 @@ int __connman_ipconfig_newaddr(int index, int family, const char *label,
 	ipaddress->prefixlen = prefixlen;
 	ipaddress->local = g_strdup(address);
 
+	if (is_index_p2p_service(index))
+		connman_ipaddress_set_p2p(ipaddress, true);
+
 	if (g_slist_find_custom(ipdevice->address_list, ipaddress,
 					check_duplicate_address)) {
 		connman_ipaddress_free(ipaddress);
@@ -1186,6 +1208,15 @@ void __connman_ipconfig_set_prefixlen(struct connman_ipconfig *ipconfig,
 	ipconfig->address->prefixlen = prefixlen;
 }
 
+static void ipconfig_set_p2p(int index, struct connman_ipconfig *ipconfig)
+{
+	if (!is_index_p2p_service(index))
+		return;
+
+	connman_ipaddress_set_p2p(ipconfig->address, true);
+	connman_ipaddress_set_p2p(ipconfig->system, true);
+}
+
 static struct connman_ipconfig *create_ipv6config(int index)
 {
 	struct connman_ipconfig *ipv6config;
@@ -1217,6 +1248,8 @@ static struct connman_ipconfig *create_ipv6config(int index)
 
 	ipv6config->system = connman_ipaddress_alloc(AF_INET6);
 
+	ipconfig_set_p2p(index, ipv6config);
+
 	DBG("ipconfig %p index %d method %s", ipv6config, index,
 		__connman_ipconfig_method2string(ipv6config->method));
 
@@ -1255,6 +1288,8 @@ struct connman_ipconfig *__connman_ipconfig_create(int index,
 
 	ipconfig->system = connman_ipaddress_alloc(AF_INET);
 
+	ipconfig_set_p2p(index, ipconfig);
+
 	DBG("ipconfig %p index %d", ipconfig, index);
 
 	return ipconfig;
@@ -1451,10 +1486,8 @@ int __connman_ipconfig_address_unset(struct connman_ipconfig *ipconfig)
 			err = connman_inet_clear_address(ipconfig->index,
 							ipconfig->address);
 		else if (ipconfig->type == CONNMAN_IPCONFIG_TYPE_IPV6)
-			err = connman_inet_clear_ipv6_address(
-						ipconfig->index,
-						ipconfig->address->local,
-						ipconfig->address->prefixlen);
+			err = connman_inet_clear_ipv6_address(ipconfig->index,
+							ipconfig->address);
 		else
 			err = -EINVAL;
 
diff --git a/src/peer.c b/src/peer.c
index 2102f119..c66b3a71 100644
--- a/src/peer.c
+++ b/src/peer.c
@@ -154,7 +154,7 @@ static int start_dhcp_server(struct connman_peer *peer)
 
 	err = __connman_inet_modify_address(RTM_NEWADDR,
 				NLM_F_REPLACE | NLM_F_ACK, index, AF_INET,
-				gateway, NULL, prefixlen, broadcast);
+				gateway, NULL, prefixlen, broadcast, true);
 	if (err < 0)
 		goto error;
 
diff --git a/src/tethering.c b/src/tethering.c
index e2687b6e..f930a26b 100644
--- a/src/tethering.c
+++ b/src/tethering.c
@@ -386,7 +386,8 @@ static void setup_tun_interface(unsigned int flags, unsigned change,
 
 	if ((__connman_inet_modify_address(RTM_NEWADDR,
 				NLM_F_REPLACE | NLM_F_ACK, pn->index, AF_INET,
-				server_ip, peer_ip, prefixlen, NULL)) < 0) {
+				server_ip, peer_ip, prefixlen, NULL, true))
+				< 0) {
 		DBG("address setting failed");
 		return;
 	}
diff --git a/vpn/plugins/l2tp.c b/vpn/plugins/l2tp.c
index 48894aa5..1e4fcd1f 100644
--- a/vpn/plugins/l2tp.c
+++ b/vpn/plugins/l2tp.c
@@ -246,6 +246,8 @@ static int l2tp_notify(DBusMessage *msg, struct vpn_provider *provider)
 		connman_ipaddress_set_ipv4(ipaddress, addressv4, netmask,
 					gateway);
 
+	connman_ipaddress_set_p2p(ipaddress, true);
+
 	vpn_provider_set_ipaddress(provider, ipaddress);
 	vpn_provider_set_nameservers(provider, nameservers);
 
diff --git a/vpn/plugins/openconnect.c b/vpn/plugins/openconnect.c
index d600e61e..4b3b1af7 100644
--- a/vpn/plugins/openconnect.c
+++ b/vpn/plugins/openconnect.c
@@ -357,6 +357,8 @@ static int oc_notify(DBusMessage *msg, struct vpn_provider *provider)
 	else
 		connman_ipaddress_set_ipv6(ipaddress, addressv6,
 						prefix_len, gateway);
+
+	connman_ipaddress_set_p2p(ipaddress, true);
 	vpn_provider_set_ipaddress(provider, ipaddress);
 	vpn_provider_set_domain(provider, domain);
 
diff --git a/vpn/plugins/openvpn.c b/vpn/plugins/openvpn.c
index f11750f5..daf66cd5 100644
--- a/vpn/plugins/openvpn.c
+++ b/vpn/plugins/openvpn.c
@@ -292,6 +292,7 @@ static int ov_notify(DBusMessage *msg, struct vpn_provider *provider)
 
 	connman_ipaddress_set_ipv4(ipaddress, address, netmask, gateway);
 	connman_ipaddress_set_peer(ipaddress, peer);
+	connman_ipaddress_set_p2p(ipaddress, true);
 	vpn_provider_set_ipaddress(provider, ipaddress);
 
 	if (nameserver_list) {
diff --git a/vpn/plugins/pptp.c b/vpn/plugins/pptp.c
index 0ede6f8b..4a704bb1 100644
--- a/vpn/plugins/pptp.c
+++ b/vpn/plugins/pptp.c
@@ -207,6 +207,7 @@ static int pptp_notify(DBusMessage *msg, struct vpn_provider *provider)
 		connman_ipaddress_set_ipv4(ipaddress, addressv4, netmask,
 					gateway);
 
+	connman_ipaddress_set_p2p(ipaddress, true);
 	vpn_provider_set_ipaddress(provider, ipaddress);
 	vpn_provider_set_nameservers(provider, nameservers);
 
diff --git a/vpn/plugins/vpnc.c b/vpn/plugins/vpnc.c
index db4b5c04..60d8266e 100644
--- a/vpn/plugins/vpnc.c
+++ b/vpn/plugins/vpnc.c
@@ -245,6 +245,7 @@ static int vc_notify(DBusMessage *msg, struct vpn_provider *provider)
 	}
 
 	connman_ipaddress_set_ipv4(ipaddress, address, netmask, gateway);
+	connman_ipaddress_set_p2p(ipaddress, true);
 	vpn_provider_set_ipaddress(provider, ipaddress);
 
 	g_free(address);
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index 7541dd6e..a0449b36 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -212,6 +212,8 @@ static int parse_address(const char *address, const char *gateway,
 		err = -EINVAL;
 	}
 
+	connman_ipaddress_set_p2p(*ipaddress, true);
+
 	g_strfreev(tokens);
 	if (err)
 		connman_ipaddress_free(*ipaddress);
diff --git a/vpn/vpn-ipconfig.c b/vpn/vpn-ipconfig.c
index c096fa37..8b792c1b 100644
--- a/vpn/vpn-ipconfig.c
+++ b/vpn/vpn-ipconfig.c
@@ -211,7 +211,7 @@ int __vpn_ipconfig_address_add(struct vpn_ipconfig *ipconfig, int family)
 
 	if (family == AF_INET)
 		return connman_inet_set_address(ipconfig->index,
-						ipconfig->address);
+							ipconfig->address);
 	else if (family == AF_INET6)
 		return connman_inet_set_ipv6_address(ipconfig->index,
 							ipconfig->address);
@@ -282,7 +282,10 @@ static struct vpn_ipconfig *create_ipv6config(int index)
 		return NULL;
 	}
 
+	connman_ipaddress_set_p2p(ipv6config->address, true);
+
 	ipv6config->system = connman_ipaddress_alloc(AF_INET6);
+	connman_ipaddress_set_p2p(ipv6config->system, true);
 
 	DBG("ipconfig %p", ipv6config);
 
@@ -314,7 +317,10 @@ struct vpn_ipconfig *__vpn_ipconfig_create(int index, int family)
 		return NULL;
 	}
 
+	connman_ipaddress_set_p2p(ipconfig->address, true);
+
 	ipconfig->system = connman_ipaddress_alloc(AF_INET);
+	connman_ipaddress_set_p2p(ipconfig->system, true);
 
 	DBG("ipconfig %p", ipconfig);
 
diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index be25e394..08728c8c 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -3068,7 +3068,7 @@ void vpn_provider_clear_address(struct vpn_provider *provider, int family)
 			DBG("ipv6 %s/%d", address, len);
 
 			connman_inet_clear_ipv6_address(provider->index,
-							address, len);
+						provider->prev_ipv6_addr);
 
 			connman_ipaddress_free(provider->prev_ipv6_addr);
 			provider->prev_ipv6_addr = NULL;