Patchwork WPS 2.0: Update authorized enrollee MAC address list correctly

login
register
mail settings
Submitter Amitkumar Karwar
Date Nov. 26, 2012, 6:02 a.m.
Message ID <5FF020A1CFFEEC49BD1E09530C4FF5950CC73D6BFA@SC-VEXCH1.marvell.com>
Download mbox | patch
Permalink /patch/201629/
State Superseded
Headers show

Comments

Amitkumar Karwar - Nov. 26, 2012, 6:02 a.m.
It is observed that authorized enrollee MAC address list advertized in
beacon and probe response sometimes contains both wildcard as well as
specific station's MAC address.

This patch makes sure that either of them will be present in the list.

Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
 src/wps/wps_registrar.c |   24 +++++++++++++++++++++---
 1 files changed, 21 insertions(+), 3 deletions(-)
Jouni Malinen - Nov. 26, 2012, 10:07 a.m.
On Sun, Nov 25, 2012 at 10:02:05PM -0800, Amitkumar Karwar wrote:
> It is observed that authorized enrollee MAC address list advertized in
> beacon and probe response sometimes contains both wildcard as well as
> specific station's MAC address.
> 
> This patch makes sure that either of them will be present in the list.

Do you mean "makes sure that _only one of them_ will be present"? That's
what the patch seemed to be doing.. And if so, why would this be desired
behavior? I would expect that a mix of wildcard and specific MAC
addresses are needed to cover some WPS use cases especially when using
multiple external Registrars.
Amitkumar Karwar - Nov. 26, 2012, 11:06 a.m.
Hi Jouni,

> > It is observed that authorized enrollee MAC address list advertized in
> > beacon and probe response sometimes contains both wildcard as well as
> > specific station's MAC address.
> >
> > This patch makes sure that either of them will be present in the list.
> 
> Do you mean "makes sure that _only one of them_ will be present"?

Yes. I will correct the patch description in updated version.

> That's
> what the patch seemed to be doing.. And if so, why would this be desired
> behavior? I would expect that a mix of wildcard and specific MAC
> addresses are needed to cover some WPS use cases especially when using
> multiple external Registrars.
> 

Mix of wildcard and specific MAC addresses were observed even for single external registrar case. This patch makes sure that AuthorizedMACs subelement prepared by an external registrar will contain either wildcard MAC or specific MAC addresses.

I assume that AP can still include both wildcard and specific MAC addresses if multiple external registrars are present.

Please correct me if I am missing something.

Thanks,
Amitkumar Karwar

Patch

diff --git a/src/wps/wps_registrar.c b/src/wps/wps_registrar.c
index d8e0d6f..d376f8d 100644
--- a/src/wps/wps_registrar.c
+++ b/src/wps/wps_registrar.c
@@ -198,6 +198,8 @@  static void wps_registrar_add_authorized_mac(struct wps_registrar *reg,
 					     const u8 *addr)
 {
 	int i;
+	u8 bcast[ETH_ALEN] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
+
 	wpa_printf(MSG_DEBUG, "WPS: Add authorized MAC " MACSTR,
 		   MAC2STR(addr));
 	for (i = 0; i < WPS_MAX_AUTHORIZED_MACS; i++)
@@ -206,9 +208,25 @@  static void wps_registrar_add_authorized_mac(struct wps_registrar *reg,
 				   "already in the list");
 			return; /* already in list */
 		}
-	for (i = WPS_MAX_AUTHORIZED_MACS - 1; i > 0; i--)
-		os_memcpy(reg->authorized_macs[i], reg->authorized_macs[i - 1],
-			  ETH_ALEN);
+
+	if (os_memcmp(addr, bcast, ETH_ALEN) == 0) {
+		/*
+		 * Clear authorized MAC address list while adding wildcard MAC
+		 */
+		for (i = 0; i < WPS_MAX_AUTHORIZED_MACS; i++)
+			os_memset(reg->authorized_macs[0], 0x00, ETH_ALEN);
+	} else {
+		/* Remove wildcard MAC when valid MAC address is added */
+		if (os_memcmp(reg->authorized_macs[0], bcast, ETH_ALEN) == 0) {
+			os_memset(reg->authorized_macs[0], 0x00, ETH_ALEN);
+		} else {
+			for (i = WPS_MAX_AUTHORIZED_MACS - 1; i > 0; i--)
+				os_memcpy(reg->authorized_macs[i],
+					  reg->authorized_macs[i - 1],
+					  ETH_ALEN);
+		}
+
+	}
 	os_memcpy(reg->authorized_macs[0], addr, ETH_ALEN);
 	wpa_hexdump(MSG_DEBUG, "WPS: Authorized MACs",
 		    (u8 *) reg->authorized_macs, sizeof(reg->authorized_macs));