WPS: Change concurrent radio AP to use only one WPS UPnP instance

WPS external Registrars can get confused about multiple UPnP
instances (one per radio) on a dual-concurrent APs. Simplify the
design by sharing a single UPnP state machine for all wireless
interfaces controlled by hostapd. This matches with the previous
changes that made a single command enable WPS functionality on
all interfaces.

This is relatively minimal change to address the sharing of the
state among multiple struct hostapd_data instances. More cleanup
can be done separately to remove unnecessary copies of information.

src/ap/wps_hostapd.c

@@ -1148,26 +1148,19 @@ static int hostapd_wps_upnp_init(struct hostapd_data *hapd,
 	if (hapd->conf->ap_pin)
 		ctx->ap_pin = os_strdup(hapd->conf->ap_pin);
 
-	hapd->wps_upnp = upnp_wps_device_init(ctx, wps, hapd);
-	if (hapd->wps_upnp == NULL) {
-		os_free(ctx);
+	hapd->wps_upnp = upnp_wps_device_init(ctx, wps, hapd,
+					      hapd->conf->upnp_iface);
+	if (hapd->wps_upnp == NULL)
 		return -1;
-	}
 	wps->wps_upnp = hapd->wps_upnp;
 
-	if (upnp_wps_device_start(hapd->wps_upnp, hapd->conf->upnp_iface)) {
-		upnp_wps_device_deinit(hapd->wps_upnp);
-		hapd->wps_upnp = NULL;
-		return -1;
-	}
-
 	return 0;
 }
 
 
 static void hostapd_wps_upnp_deinit(struct hostapd_data *hapd)
 {
-	upnp_wps_device_deinit(hapd->wps_upnp);
+	upnp_wps_device_deinit(hapd->wps_upnp, hapd);
 }
 
 #endif /* CONFIG_WPS_UPNP */

src/wps/wps_upnp.c

@@ -212,6 +212,10 @@
 /* Maximum number of Probe Request events per second */
 #define MAX_EVENTS_PER_SEC 5
 
+
+static struct upnp_wps_device_sm *shared_upnp_device = NULL;
+
+
 /* Write the current date/time per RFC */
 void format_date(struct wpabuf *buf)
 {
@@ -965,7 +969,7 @@ static void upnp_wps_free_subscriptions(struct dl_list *head)
  * upnp_wps_device_stop - Stop WPS UPnP operations on an interface
  * @sm: WPS UPnP state machine from upnp_wps_device_init()
  */
-void upnp_wps_device_stop(struct upnp_wps_device_sm *sm)
+static void upnp_wps_device_stop(struct upnp_wps_device_sm *sm)
 {
 	if (!sm || !sm->started)
 		return;
@@ -997,7 +1001,7 @@ void upnp_wps_device_stop(struct upnp_wps_device_sm *sm)
  * @net_if: Selected network interface name
  * Returns: 0 on success, -1 on failure
  */
-int upnp_wps_device_start(struct upnp_wps_device_sm *sm, char *net_if)
+static int upnp_wps_device_start(struct upnp_wps_device_sm *sm, char *net_if)
 {
 	if (!sm || !net_if)
 		return -1;
@@ -1052,24 +1056,56 @@ fail:
 }
 
 
+static struct upnp_wps_device_interface *
+upnp_wps_get_iface(struct upnp_wps_device_sm *sm, void *priv)
+{
+	struct upnp_wps_device_interface *iface;
+	dl_list_for_each(iface, &sm->interfaces,
+			 struct upnp_wps_device_interface, list) {
+		if (iface->priv == priv)
+			return iface;
+	}
+	return NULL;
+}
+
+
 /**
  * upnp_wps_device_deinit - Deinitialize WPS UPnP
  * @sm: WPS UPnP state machine from upnp_wps_device_init()
+ * @priv: External context data that was used in upnp_wps_device_init() call
  */
-void upnp_wps_device_deinit(struct upnp_wps_device_sm *sm)
+void upnp_wps_device_deinit(struct upnp_wps_device_sm *sm, void *priv)
 {
+	struct upnp_wps_device_interface *iface;
+
 	if (!sm)
 		return;
 
-	upnp_wps_device_stop(sm);
-
-	if (sm->peer.wps)
-		wps_deinit(sm->peer.wps);
-	os_free(sm->root_dir);
-	os_free(sm->desc_url);
-	os_free(sm->ctx->ap_pin);
-	os_free(sm->ctx);
-	os_free(sm);
+	iface = upnp_wps_get_iface(sm, priv);
+	if (iface == NULL) {
+		wpa_printf(MSG_ERROR, "WPS UPnP: Could not find the interface "
+			   "instance to deinit");
+		return;
+	}
+	wpa_printf(MSG_DEBUG, "WPS UPnP: Deinit interface instance %p", iface);
+	if (dl_list_len(&sm->interfaces) == 1) {
+		wpa_printf(MSG_DEBUG, "WPS UPnP: Deinitializing last instance "
+			   "- free global device instance");
+		upnp_wps_device_stop(sm);
+	}
+	dl_list_del(&iface->list);
+
+	if (iface->peer.wps)
+		wps_deinit(iface->peer.wps);
+	os_free(iface->ctx->ap_pin);
+	os_free(iface->ctx);
+
+	if (dl_list_empty(&sm->interfaces)) {
+		os_free(sm->root_dir);
+		os_free(sm->desc_url);
+		os_free(sm);
+		shared_upnp_device = NULL;
+	}
 }
 
 
@@ -1078,25 +1114,59 @@ void upnp_wps_device_deinit(struct upnp_wps_device_sm *sm)
  * @ctx: callback table; we must eventually free it
  * @wps: Pointer to longterm WPS context
  * @priv: External context data that will be used in callbacks
+ * @net_if: Selected network interface name
  * Returns: WPS UPnP state or %NULL on failure
  */
 struct upnp_wps_device_sm *
 upnp_wps_device_init(struct upnp_wps_device_ctx *ctx, struct wps_context *wps,
-		     void *priv)
+		     void *priv, char *net_if)
 {
 	struct upnp_wps_device_sm *sm;
+	struct upnp_wps_device_interface *iface;
+	int start = 0;
+
+	iface = os_zalloc(sizeof(*iface));
+	if (iface == NULL) {
+		os_free(ctx->ap_pin);
+		os_free(ctx);
+		return NULL;
+	}
+	wpa_printf(MSG_DEBUG, "WPS UPnP: Init interface instance %p", iface);
+
+	iface->ctx = ctx;
+	iface->wps = wps;
+	iface->priv = priv;
 
-	sm = os_zalloc(sizeof(*sm));
-	if (!sm) {
-		wpa_printf(MSG_ERROR, "WPS UPnP: upnp_wps_device_init failed");
+	if (shared_upnp_device) {
+		wpa_printf(MSG_DEBUG, "WPS UPnP: Share existing device "
+			   "context");
+		sm = shared_upnp_device;
+	} else {
+		wpa_printf(MSG_DEBUG, "WPS UPnP: Initialize device context");
+		sm = os_zalloc(sizeof(*sm));
+		if (!sm) {
+			wpa_printf(MSG_ERROR, "WPS UPnP: upnp_wps_device_init "
+				   "failed");
+			os_free(iface);
+			os_free(ctx->ap_pin);
+			os_free(ctx);
+			return NULL;
+		}
+		shared_upnp_device = sm;
+
+		dl_list_init(&sm->msearch_replies);
+		dl_list_init(&sm->subscriptions);
+		dl_list_init(&sm->interfaces);
+		start = 1;
+	}
+
+	dl_list_add(&sm->interfaces, &iface->list);
+
+	if (start && upnp_wps_device_start(sm, net_if)) {
+		upnp_wps_device_deinit(sm, priv);
 		return NULL;
 	}
 
-	sm->ctx = ctx;
-	sm->wps = wps;
-	sm->priv = priv;
-	dl_list_init(&sm->msearch_replies);
-	dl_list_init(&sm->subscriptions);
 
 	return sm;
 }
@@ -1115,16 +1185,20 @@ int upnp_wps_subscribers(struct upnp_wps_device_sm *sm)
 
 int upnp_wps_set_ap_pin(struct upnp_wps_device_sm *sm, const char *ap_pin)
 {
+	struct upnp_wps_device_interface *iface;
 	if (sm == NULL)
 		return 0;
 
-	os_free(sm->ctx->ap_pin);
-	if (ap_pin) {
-		sm->ctx->ap_pin = os_strdup(ap_pin);
-		if (sm->ctx->ap_pin == NULL)
-			return -1;
-	} else
-		sm->ctx->ap_pin = NULL;
+	dl_list_for_each(iface, &sm->interfaces,
+			 struct upnp_wps_device_interface, list) {
+		os_free(iface->ctx->ap_pin);
+		if (ap_pin) {
+			iface->ctx->ap_pin = os_strdup(ap_pin);
+			if (iface->ctx->ap_pin == NULL)
+				return -1;
+		} else
+			iface->ctx->ap_pin = NULL;
+	}
 
 	return 0;
 }

src/wps/wps_upnp.h

@@ -35,11 +35,8 @@ struct upnp_wps_device_ctx {
 
 struct upnp_wps_device_sm *
 upnp_wps_device_init(struct upnp_wps_device_ctx *ctx, struct wps_context *wps,
-		     void *priv);
-void upnp_wps_device_deinit(struct upnp_wps_device_sm *sm);
-
-int upnp_wps_device_start(struct upnp_wps_device_sm *sm, char *net_if);
-void upnp_wps_device_stop(struct upnp_wps_device_sm *sm);
+		     void *priv, char *net_if);
+void upnp_wps_device_deinit(struct upnp_wps_device_sm *sm, void *priv);
 
 int upnp_wps_device_send_wlan_event(struct upnp_wps_device_sm *sm,
 				    const u8 from_mac_addr[ETH_ALEN],

src/wps/wps_upnp_i.h

@@ -103,16 +103,26 @@ struct subscription {
 };
 
 
+struct upnp_wps_device_interface {
+	struct dl_list list;
+	struct upnp_wps_device_ctx *ctx; /* callback table */
+	struct wps_context *wps;
+	void *priv;
+
+	/* FIX: maintain separate structures for each UPnP peer */
+	struct upnp_wps_peer peer;
+};
+
 /*
- * Our instance data corresponding to one WiFi network interface
- * (multiple might share the same wired network interface!).
+ * Our instance data corresponding to the AP device. Note that there may be
+ * multiple wireless interfaces sharing the same UPnP device instance. Each
+ * such interface is stored in the list of struct upnp_wps_device_interface
+ * instances.
  *
  * This is known as an opaque struct declaration to users of the WPS UPnP code.
  */
 struct upnp_wps_device_sm {
-	struct upnp_wps_device_ctx *ctx; /* callback table */
-	struct wps_context *wps;
-	void *priv;
+	struct dl_list interfaces; /* struct upnp_wps_device_interface */
 	char *root_dir;
 	char *desc_url;
 	int started; /* nonzero if we are active */
@@ -136,9 +146,6 @@ struct upnp_wps_device_sm {
 	enum upnp_wps_wlanevent_type wlanevent_type;
 	os_time_t last_event_sec;
 	unsigned int num_events_in_sec;
-
-	/* FIX: maintain separate structures for each UPnP peer */
-	struct upnp_wps_peer peer;
 };
 
 /* wps_upnp.c */

src/wps/wps_upnp_ssdp.c

@@ -136,9 +136,12 @@ next_advertisement(struct upnp_wps_device_sm *sm,
 	struct wpabuf *msg;
 	char *NTString = "";
 	char uuid_string[80];
+	struct upnp_wps_device_interface *iface;
 
 	*islast = 0;
-	uuid_bin2str(sm->wps->uuid, uuid_string, sizeof(uuid_string));
+	iface = dl_list_first(&sm->interfaces,
+			      struct upnp_wps_device_interface, list);
+	uuid_bin2str(iface->wps->uuid, uuid_string, sizeof(uuid_string));
 	msg = wpabuf_alloc(800); /* more than big enough */
 	if (msg == NULL)
 		goto fail;
@@ -588,8 +591,13 @@ static void ssdp_parse_msearch(struct upnp_wps_device_sm *sm,
 			}
 			if (str_starts(data, "uuid:")) {
 				char uuid_string[80];
+				struct upnp_wps_device_interface *iface;
+				iface = dl_list_first(
+					&sm->interfaces,
+					struct upnp_wps_device_interface,
+					list);
 				data += os_strlen("uuid:");
-				uuid_bin2str(sm->wps->uuid, uuid_string,
+				uuid_bin2str(iface->wps->uuid, uuid_string,
 					     sizeof(uuid_string));
 				if (str_starts(data, uuid_string))
 					st_match = 1;

src/wps/wps_upnp_web.c

@@ -184,6 +184,10 @@ static void format_wps_device_xml(struct upnp_wps_device_sm *sm,
 {
 	const char *s;
 	char uuid_string[80];
+	struct upnp_wps_device_interface *iface;
+
+	iface = dl_list_first(&sm->interfaces,
+			      struct upnp_wps_device_interface, list);
 
 	wpabuf_put_str(buf, wps_device_xml_prefix);
 
@@ -191,38 +195,38 @@ static void format_wps_device_xml(struct upnp_wps_device_sm *sm,
 	 * Add required fields with default values if not configured. Add
 	 * optional and recommended fields only if configured.
 	 */
-	s = sm->wps->friendly_name;
+	s = iface->wps->friendly_name;
 	s = ((s && *s) ? s : "WPS Access Point");
 	xml_add_tagged_data(buf, "friendlyName", s);
 
-	s = sm->wps->dev.manufacturer;
+	s = iface->wps->dev.manufacturer;
 	s = ((s && *s) ? s : "");
 	xml_add_tagged_data(buf, "manufacturer", s);
 
-	if (sm->wps->manufacturer_url)
+	if (iface->wps->manufacturer_url)
 		xml_add_tagged_data(buf, "manufacturerURL",
-				    sm->wps->manufacturer_url);
+				    iface->wps->manufacturer_url);
 
-	if (sm->wps->model_description)
+	if (iface->wps->model_description)
 		xml_add_tagged_data(buf, "modelDescription",
-				    sm->wps->model_description);
+				    iface->wps->model_description);
 
-	s = sm->wps->dev.model_name;
+	s = iface->wps->dev.model_name;
 	s = ((s && *s) ? s : "");
 	xml_add_tagged_data(buf, "modelName", s);
 
-	if (sm->wps->dev.model_number)
+	if (iface->wps->dev.model_number)
 		xml_add_tagged_data(buf, "modelNumber",
-				    sm->wps->dev.model_number);
+				    iface->wps->dev.model_number);
 
-	if (sm->wps->model_url)
-		xml_add_tagged_data(buf, "modelURL", sm->wps->model_url);
+	if (iface->wps->model_url)
+		xml_add_tagged_data(buf, "modelURL", iface->wps->model_url);
 
-	if (sm->wps->dev.serial_number)
+	if (iface->wps->dev.serial_number)
 		xml_add_tagged_data(buf, "serialNumber",
-				    sm->wps->dev.serial_number);
+				    iface->wps->dev.serial_number);
 
-	uuid_bin2str(sm->wps->uuid, uuid_string, sizeof(uuid_string));
+	uuid_bin2str(iface->wps->uuid, uuid_string, sizeof(uuid_string));
 	s = uuid_string;
 	/* Need "uuid:" prefix, thus we can't use xml_add_tagged_data()
 	 * easily...
@@ -231,8 +235,8 @@ static void format_wps_device_xml(struct upnp_wps_device_sm *sm,
 	xml_data_encode(buf, s, os_strlen(s));
 	wpabuf_put_str(buf, "</UDN>\n");
 
-	if (sm->wps->upc)
-		xml_add_tagged_data(buf, "UPC", sm->wps->upc);
+	if (iface->wps->upc)
+		xml_add_tagged_data(buf, "UPC", iface->wps->upc);
 
 	wpabuf_put_str(buf, wps_device_xml_postfix);
 }
@@ -311,6 +315,10 @@ static void web_connection_parse_get(struct upnp_wps_device_sm *sm,
 	size_t extra_len = 0;
 	int body_length;
 	char len_buf[10];
+	struct upnp_wps_device_interface *iface;
+
+	iface = dl_list_first(&sm->interfaces,
+			      struct upnp_wps_device_interface, list);
 
 	/*
 	 * It is not required that filenames be case insensitive but it is
@@ -322,16 +330,16 @@ static void web_connection_parse_get(struct upnp_wps_device_sm *sm,
 		wpa_printf(MSG_DEBUG, "WPS UPnP: HTTP GET for device XML");
 		req = GET_DEVICE_XML_FILE;
 		extra_len = 3000;
-		if (sm->wps->friendly_name)
-			extra_len += os_strlen(sm->wps->friendly_name);
-		if (sm->wps->manufacturer_url)
-			extra_len += os_strlen(sm->wps->manufacturer_url);
-		if (sm->wps->model_description)
-			extra_len += os_strlen(sm->wps->model_description);
-		if (sm->wps->model_url)
-			extra_len += os_strlen(sm->wps->model_url);
-		if (sm->wps->upc)
-			extra_len += os_strlen(sm->wps->upc);
+		if (iface->wps->friendly_name)
+			extra_len += os_strlen(iface->wps->friendly_name);
+		if (iface->wps->manufacturer_url)
+			extra_len += os_strlen(iface->wps->manufacturer_url);
+		if (iface->wps->model_description)
+			extra_len += os_strlen(iface->wps->model_description);
+		if (iface->wps->model_url)
+			extra_len += os_strlen(iface->wps->model_url);
+		if (iface->wps->upc)
+			extra_len += os_strlen(iface->wps->upc);
 	} else if (!os_strcasecmp(filename, UPNP_WPS_SCPD_XML_FILE)) {
 		wpa_printf(MSG_DEBUG, "WPS UPnP: HTTP GET for SCPD XML");
 		req = GET_SCPD_XML_FILE;
@@ -408,11 +416,16 @@ web_process_get_device_info(struct upnp_wps_device_sm *sm,
 {
 	static const char *name = "NewDeviceInfo";
 	struct wps_config cfg;
-	struct upnp_wps_peer *peer = &sm->peer;
+	struct upnp_wps_device_interface *iface;
+	struct upnp_wps_peer *peer;
+
+	iface = dl_list_first(&sm->interfaces,
+			      struct upnp_wps_device_interface, list);
+	peer = &iface->peer;
 
 	wpa_printf(MSG_DEBUG, "WPS UPnP: GetDeviceInfo");
 
-	if (sm->ctx->ap_pin == NULL)
+	if (iface->ctx->ap_pin == NULL)
 		return HTTP_INTERNAL_SERVER_ERROR;
 
 	/*
@@ -427,9 +440,9 @@ web_process_get_device_info(struct upnp_wps_device_sm *sm,
 		wps_deinit(peer->wps);
 
 	os_memset(&cfg, 0, sizeof(cfg));
-	cfg.wps = sm->wps;
-	cfg.pin = (u8 *) sm->ctx->ap_pin;
-	cfg.pin_len = os_strlen(sm->ctx->ap_pin);
+	cfg.wps = iface->wps;
+	cfg.pin = (u8 *) iface->ctx->ap_pin;
+	cfg.pin_len = os_strlen(iface->ctx->ap_pin);
 	peer->wps = wps_init(&cfg);
 	if (peer->wps) {
 		enum wsc_op_code op_code;
@@ -458,6 +471,10 @@ web_process_put_message(struct upnp_wps_device_sm *sm, char *data,
 	enum http_reply_code ret;
 	enum wps_process_res res;
 	enum wsc_op_code op_code;
+	struct upnp_wps_device_interface *iface;
+
+	iface = dl_list_first(&sm->interfaces,
+			      struct upnp_wps_device_interface, list);
 
 	/*
 	 * PutMessage is used by external UPnP-based Registrar to perform WPS
@@ -468,11 +485,11 @@ web_process_put_message(struct upnp_wps_device_sm *sm, char *data,
 	msg = xml_get_base64_item(data, "NewInMessage", &ret);
 	if (msg == NULL)
 		return ret;
-	res = wps_process_msg(sm->peer.wps, WSC_UPnP, msg);
+	res = wps_process_msg(iface->peer.wps, WSC_UPnP, msg);
 	if (res == WPS_FAILURE)
 		*reply = NULL;
 	else
-		*reply = wps_get_msg(sm->peer.wps, &op_code);
+		*reply = wps_get_msg(iface->peer.wps, &op_code);
 	wpabuf_free(msg);
 	if (*reply == NULL)
 		return HTTP_INTERNAL_SERVER_ERROR;
@@ -491,6 +508,8 @@ web_process_put_wlan_response(struct upnp_wps_device_sm *sm, char *data,
 	int ev_type;
 	int type;
 	char *val;
+	struct upnp_wps_device_interface *iface;
+	int ok = 0;
 
 	/*
 	 * External UPnP-based Registrar is passing us a message to be proxied
@@ -559,9 +578,16 @@ web_process_put_wlan_response(struct upnp_wps_device_sm *sm, char *data,
 		wpa_printf(MSG_DEBUG, "WPS UPnP: Message Type %d", type);
 	} else
 		type = -1;
-	if (!sm->ctx->rx_req_put_wlan_response ||
-	    sm->ctx->rx_req_put_wlan_response(sm->priv, ev_type, macaddr, msg,
-					      type)) {
+	dl_list_for_each(iface, &sm->interfaces,
+			 struct upnp_wps_device_interface, list) {
+		if (iface->ctx->rx_req_put_wlan_response &&
+		    iface->ctx->rx_req_put_wlan_response(iface->priv, ev_type,
+							 macaddr, msg, type)
+		    == 0)
+			ok = 1;
+	}
+
+	if (!ok) {
 		wpa_printf(MSG_INFO, "WPS UPnP: Fail: sm->ctx->"
 			   "rx_req_put_wlan_response");
 		wpabuf_free(msg);
@@ -606,6 +632,8 @@ web_process_set_selected_registrar(struct upnp_wps_device_sm *sm,
 	struct wpabuf *msg;
 	enum http_reply_code ret;
 	struct subscription *s;
+	struct upnp_wps_device_interface *iface;
+	int err = 0;
 
 	wpa_printf(MSG_DEBUG, "WPS UPnP: SetSelectedRegistrar");
 	s = find_er(sm, cli);
@@ -617,11 +645,15 @@ web_process_set_selected_registrar(struct upnp_wps_device_sm *sm,
 	msg = xml_get_base64_item(data, "NewMessage", &ret);
 	if (msg == NULL)
 		return ret;
-	if (upnp_er_set_selected_registrar(sm->wps->registrar, s, msg)) {
-		wpabuf_free(msg);
-		return HTTP_INTERNAL_SERVER_ERROR;
+	dl_list_for_each(iface, &sm->interfaces,
+			 struct upnp_wps_device_interface, list) {
+		if (upnp_er_set_selected_registrar(iface->wps->registrar, s,
+						   msg))
+			err = 1;
 	}
 	wpabuf_free(msg);
+	if (err)
+		return HTTP_INTERNAL_SERVER_ERROR;
 	*replyname = NULL;
 	*reply = NULL;
 	return HTTP_OK;