WPS 2.0: Update authorized enrollee MAC address list correctly

Submitted by Amitkumar Karwar on Nov. 26, 2012, 6:02 a.m.

Details

Message ID 5FF020A1CFFEEC49BD1E09530C4FF5950CC73D6BFA@SC-VEXCH1.marvell.com
State Superseded
Headers show

Commit Message

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(-)

Comments

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 hide | download patch | download mbox

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));