[v2] wpa_supplicant: fix auth failure when the MAC is updated externally

Message ID 20180215105001.19229-1-bgalvani@redhat.com
State Accepted
Headers show
Series
  • [v2] wpa_supplicant: fix auth failure when the MAC is updated externally
Related show

Commit Message

Beniamino Galvani Feb. 15, 2018, 10:50 a.m.
When connecting to a WPA-EAP network and the MAC address is changed
just before the association (for example by NetworkManager, which sets
a random MAC during scans), the authentication sometimes fails in the
following way ('####' logs added by me):

wpa_supplicant logs:
 wlan0: WPA: RX message 1 of 4-Way Handshake from 02:00:00:00:01:00 (ver=1)
 RSN: msg 1/4 key data - hexdump(len=22): dd 14 00 0f ac 04 d8 21 9d a5 73 98 88 26 ef 03 d2 ce f7 04 7d 23
 WPA: PMKID in EAPOL-Key - hexdump(len=22): dd 14 00 0f ac 04 d8 21 9d a5 73 98 88 26 ef 03 d2 ce f7 04 7d 23
 RSN: PMKID from Authenticator - hexdump(len=16): d8 21 9d a5 73 98 88 26 ef 03 d2 ce f7 04 7d 23
 wlan0: RSN: no matching PMKID found
 EAPOL: Successfully fetched key (len=32)
 WPA: PMK from EAPOL state machines - hexdump(len=32): [REMOVED]
 #### WPA: rsn_pmkid():
 #### WPA: aa              - hexdump(len=6): 02 00 00 00 01 00
 #### WPA: spa             - hexdump(len=6): 66 20 cf ab 8c dc
 #### WPA: PMK             - hexdump(len=32): b5 24 76 4f 6f 50 8c f6 a1 2e 24 b8 07 4e 9a 13 1b 94 c4 a8 1f 7e 22 d6 ed fc 7d 43 c7 77 b6 f7
 #### WPA: computed PMKID  - hexdump(len=16): ea 73 67 b1 8e 5f 18 43 58 24 e8 1c 47 23 87 71
 RSN: Replace PMKSA entry for the current AP and any PMKSA cache entry that was based on the old PMK
 nl80211: Delete PMKID for 02:00:00:00:01:00
 wlan0: RSN: PMKSA cache entry free_cb: 02:00:00:00:01:00 reason=1
 RSN: Added PMKSA cache entry for 02:00:00:00:01:00 network_ctx=0x5630bf85a270
 nl80211: Add PMKID for 02:00:00:00:01:00
 wlan0: RSN: PMKID mismatch - authentication server may have derived different MSK?!

hostapd logs:
 WPA: PMK from EAPOL state machine (MSK len=64 PMK len=32)
 WPA: 02:00:00:00:00:00 WPA_PTK entering state PTKSTART
 wlan1: STA 02:00:00:00:00:00 WPA: sending 1/4 msg of 4-Way Handshake
 #### WPA: rsn_pmkid():
 #### WPA: aa              - hexdump(len=6): 02 00 00 00 01 00
 #### WPA: spa             - hexdump(len=6): 02 00 00 00 00 00
 #### WPA: PMK             - hexdump(len=32): b5 24 76 4f 6f 50 8c f6 a1 2e 24 b8 07 4e 9a 13 1b 94 c4 a8 1f 7e 22 d6 ed fc 7d 43 c7 77 b6 f7
 #### WPA: computed PMKID  - hexdump(len=16): d8 21 9d a5 73 98 88 26 ef 03 d2 ce f7 04 7d 23
 WPA: Send EAPOL(version=1 secure=0 mic=0 ack=1 install=0 pairwise=1 kde_len=22 keyidx=0 encr=0)

That's because wpa_supplicant computed the PMKID using the wrong (old)
MAC address used during the scan. wpa_supplicant updates own_addr when
the interface goes up, as the MAC can only change while the interface
is down. However, drivers don't report all interface state changes:
for example the nl80211 driver may ignore a down-up cycle if the down
message is processed later, when the interface is already up. In such
cases, wpa_supplicant (and in particular, the EAP state machine) would
continue to use the old MAC.

Add a new driver event that notifies of MAC address changes while the
interface is active.

Signed-off-by: Beniamino Galvani <bgalvani@redhat.com>
---

Changes since v1 at http://lists.infradead.org/pipermail/hostap/2017-September/037936.html
 - reworded commit message

 src/drivers/driver.h         |  9 +++++++++
 src/drivers/driver_common.c  |  1 +
 src/drivers/driver_nl80211.c | 13 +++++++++----
 wpa_supplicant/events.c      |  3 +++
 4 files changed, 22 insertions(+), 4 deletions(-)

Comments

Dan Williams March 20, 2018, 9:31 p.m. | #1
On Thu, 2018-02-15 at 11:50 +0100, Beniamino Galvani wrote:
> When connecting to a WPA-EAP network and the MAC address is changed
> just before the association (for example by NetworkManager, which
> sets
> a random MAC during scans), the authentication sometimes fails in the
> following way ('####' logs added by me):

Patch looks reasonable to me.  Jouni, any thoughts on this one?

Thanks!
Dan

> wpa_supplicant logs:
>  wlan0: WPA: RX message 1 of 4-Way Handshake from 02:00:00:00:01:00
> (ver=1)
>  RSN: msg 1/4 key data - hexdump(len=22): dd 14 00 0f ac 04 d8 21 9d
> a5 73 98 88 26 ef 03 d2 ce f7 04 7d 23
>  WPA: PMKID in EAPOL-Key - hexdump(len=22): dd 14 00 0f ac 04 d8 21
> 9d a5 73 98 88 26 ef 03 d2 ce f7 04 7d 23
>  RSN: PMKID from Authenticator - hexdump(len=16): d8 21 9d a5 73 98
> 88 26 ef 03 d2 ce f7 04 7d 23
>  wlan0: RSN: no matching PMKID found
>  EAPOL: Successfully fetched key (len=32)
>  WPA: PMK from EAPOL state machines - hexdump(len=32): [REMOVED]
>  #### WPA: rsn_pmkid():
>  #### WPA: aa              - hexdump(len=6): 02 00 00 00 01 00
>  #### WPA: spa             - hexdump(len=6): 66 20 cf ab 8c dc
>  #### WPA: PMK             - hexdump(len=32): b5 24 76 4f 6f 50 8c f6
> a1 2e 24 b8 07 4e 9a 13 1b 94 c4 a8 1f 7e 22 d6 ed fc 7d 43 c7 77 b6
> f7
>  #### WPA: computed PMKID  - hexdump(len=16): ea 73 67 b1 8e 5f 18 43
> 58 24 e8 1c 47 23 87 71
>  RSN: Replace PMKSA entry for the current AP and any PMKSA cache
> entry that was based on the old PMK
>  nl80211: Delete PMKID for 02:00:00:00:01:00
>  wlan0: RSN: PMKSA cache entry free_cb: 02:00:00:00:01:00 reason=1
>  RSN: Added PMKSA cache entry for 02:00:00:00:01:00
> network_ctx=0x5630bf85a270
>  nl80211: Add PMKID for 02:00:00:00:01:00
>  wlan0: RSN: PMKID mismatch - authentication server may have derived
> different MSK?!
> 
> hostapd logs:
>  WPA: PMK from EAPOL state machine (MSK len=64 PMK len=32)
>  WPA: 02:00:00:00:00:00 WPA_PTK entering state PTKSTART
>  wlan1: STA 02:00:00:00:00:00 WPA: sending 1/4 msg of 4-Way Handshake
>  #### WPA: rsn_pmkid():
>  #### WPA: aa              - hexdump(len=6): 02 00 00 00 01 00
>  #### WPA: spa             - hexdump(len=6): 02 00 00 00 00 00
>  #### WPA: PMK             - hexdump(len=32): b5 24 76 4f 6f 50 8c f6
> a1 2e 24 b8 07 4e 9a 13 1b 94 c4 a8 1f 7e 22 d6 ed fc 7d 43 c7 77 b6
> f7
>  #### WPA: computed PMKID  - hexdump(len=16): d8 21 9d a5 73 98 88 26
> ef 03 d2 ce f7 04 7d 23
>  WPA: Send EAPOL(version=1 secure=0 mic=0 ack=1 install=0 pairwise=1
> kde_len=22 keyidx=0 encr=0)
> 
> That's because wpa_supplicant computed the PMKID using the wrong
> (old)
> MAC address used during the scan. wpa_supplicant updates own_addr
> when
> the interface goes up, as the MAC can only change while the interface
> is down. However, drivers don't report all interface state changes:
> for example the nl80211 driver may ignore a down-up cycle if the down
> message is processed later, when the interface is already up. In such
> cases, wpa_supplicant (and in particular, the EAP state machine)
> would
> continue to use the old MAC.
> 
> Add a new driver event that notifies of MAC address changes while the
> interface is active.
> 
> Signed-off-by: Beniamino Galvani <bgalvani@redhat.com>
> ---
> 
> Changes since v1 at http://lists.infradead.org/pipermail/hostap/2017-
> September/037936.html
>  - reworded commit message
> 
>  src/drivers/driver.h         |  9 +++++++++
>  src/drivers/driver_common.c  |  1 +
>  src/drivers/driver_nl80211.c | 13 +++++++++----
>  wpa_supplicant/events.c      |  3 +++
>  4 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/src/drivers/driver.h b/src/drivers/driver.h
> index d9c6fd9f5..c250c11f6 100644
> --- a/src/drivers/driver.h
> +++ b/src/drivers/driver.h
> @@ -4307,6 +4307,15 @@ enum wpa_event_type {
>  	 */
>  	EVENT_SIGNAL_CHANGE,
>  
> +	/**
> +	 * EVENT_INTERFACE_MAC_CHANGED - Notify that interface MAC
> changed
> +	 *
> +	 * This event is emitted when the MAC changes while the
> interface is
> +	 * enabled. When an interface was disabled and becomes
> enabled, it
> +	 * must be always assumed that the MAC possibly changed.
> +	 */
> +	EVENT_INTERFACE_MAC_CHANGED,
> +
>  	/**
>  	 * EVENT_INTERFACE_ENABLED - Notify that interface was
> enabled
>  	 *
> diff --git a/src/drivers/driver_common.c
> b/src/drivers/driver_common.c
> index 33a6db346..c91d66ab5 100644
> --- a/src/drivers/driver_common.c
> +++ b/src/drivers/driver_common.c
> @@ -53,6 +53,7 @@ const char * event_to_string(enum wpa_event_type
> event)
>  	E2S(NEW_STA);
>  	E2S(EAPOL_RX);
>  	E2S(SIGNAL_CHANGE);
> +	E2S(INTERFACE_MAC_CHANGED);
>  	E2S(INTERFACE_ENABLED);
>  	E2S(INTERFACE_DISABLED);
>  	E2S(CHANNEL_LIST_CHANGED);
> diff --git a/src/drivers/driver_nl80211.c
> b/src/drivers/driver_nl80211.c
> index dfa11834c..13f5959b6 100644
> --- a/src/drivers/driver_nl80211.c
> +++ b/src/drivers/driver_nl80211.c
> @@ -951,7 +951,7 @@ nl80211_find_drv(struct nl80211_global *global,
> int idx, u8 *buf, size_t len,
>  
>  
>  static void nl80211_refresh_mac(struct wpa_driver_nl80211_data *drv,
> -				int ifindex)
> +				int ifindex, Boolean notify)
>  {
>  	struct i802_bss *bss;
>  	u8 addr[ETH_ALEN];
> @@ -970,6 +970,11 @@ static void nl80211_refresh_mac(struct
> wpa_driver_nl80211_data *drv,
>  			   ifindex, bss->ifname,
>  			   MAC2STR(bss->addr), MAC2STR(addr));
>  		os_memcpy(bss->addr, addr, ETH_ALEN);
> +		if (notify) {
> +			wpa_supplicant_event(drv->ctx,
> +					     EVENT_INTERFACE_MAC_CHA
> NGED,
> +					     NULL);
> +		}
>  	}
>  }
>  
> @@ -1041,11 +1046,11 @@ static void
> wpa_driver_nl80211_event_rtm_newlink(void *ctx,
>  		namebuf[0] = '\0';
>  		if (if_indextoname(ifi->ifi_index, namebuf) &&
>  		    linux_iface_up(drv->global->ioctl_sock, namebuf)
> > 0) {
> -			/* Re-read MAC address as it may have
> changed */
> -			nl80211_refresh_mac(drv, ifi->ifi_index);
>  			wpa_printf(MSG_DEBUG, "nl80211: Ignore
> interface down "
>  				   "event since interface %s is up",
> namebuf);
>  			drv->ignore_if_down_event = 0;
> +			/* Re-read MAC address as it may have
> changed */
> +			nl80211_refresh_mac(drv, ifi->ifi_index,
> TRUE);
>  			return;
>  		}
>  		wpa_printf(MSG_DEBUG, "nl80211: Interface down
> (%s/%s)",
> @@ -1091,7 +1096,7 @@ static void
> wpa_driver_nl80211_event_rtm_newlink(void *ctx,
>  				   "removed", drv->first_bss-
> >ifname);
>  		} else {
>  			/* Re-read MAC address as it may have
> changed */
> -			nl80211_refresh_mac(drv, ifi->ifi_index);
> +			nl80211_refresh_mac(drv, ifi->ifi_index,
> FALSE);
>  
>  			wpa_printf(MSG_DEBUG, "nl80211: Interface
> up");
>  			drv->if_disabled = 0;
> diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
> index 4f3604358..e8cbac548 100644
> --- a/wpa_supplicant/events.c
> +++ b/wpa_supplicant/events.c
> @@ -4366,6 +4366,9 @@ void wpa_supplicant_event(void *ctx, enum
> wpa_event_type event,
>  			data->signal_change.current_noise,
>  			data->signal_change.current_txrate);
>  		break;
> +	case EVENT_INTERFACE_MAC_CHANGED:
> +		wpa_supplicant_update_mac_addr(wpa_s);
> +		break;
>  	case EVENT_INTERFACE_ENABLED:
>  		wpa_dbg(wpa_s, MSG_DEBUG, "Interface was enabled");
>  		if (wpa_s->wpa_state == WPA_INTERFACE_DISABLED) {
Jouni Malinen March 30, 2018, 1:31 p.m. | #2
On Thu, Feb 15, 2018 at 11:50:01AM +0100, Beniamino Galvani wrote:
> When connecting to a WPA-EAP network and the MAC address is changed
> just before the association (for example by NetworkManager, which sets
> a random MAC during scans), the authentication sometimes fails in the
> following way ('####' logs added by me):
...
> That's because wpa_supplicant computed the PMKID using the wrong (old)
> MAC address used during the scan. wpa_supplicant updates own_addr when
> the interface goes up, as the MAC can only change while the interface
> is down. However, drivers don't report all interface state changes:
> for example the nl80211 driver may ignore a down-up cycle if the down
> message is processed later, when the interface is already up. In such
> cases, wpa_supplicant (and in particular, the EAP state machine) would
> continue to use the old MAC.
> 
> Add a new driver event that notifies of MAC address changes while the
> interface is active.

Thanks, applied.

Patch

diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index d9c6fd9f5..c250c11f6 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -4307,6 +4307,15 @@  enum wpa_event_type {
 	 */
 	EVENT_SIGNAL_CHANGE,
 
+	/**
+	 * EVENT_INTERFACE_MAC_CHANGED - Notify that interface MAC changed
+	 *
+	 * This event is emitted when the MAC changes while the interface is
+	 * enabled. When an interface was disabled and becomes enabled, it
+	 * must be always assumed that the MAC possibly changed.
+	 */
+	EVENT_INTERFACE_MAC_CHANGED,
+
 	/**
 	 * EVENT_INTERFACE_ENABLED - Notify that interface was enabled
 	 *
diff --git a/src/drivers/driver_common.c b/src/drivers/driver_common.c
index 33a6db346..c91d66ab5 100644
--- a/src/drivers/driver_common.c
+++ b/src/drivers/driver_common.c
@@ -53,6 +53,7 @@  const char * event_to_string(enum wpa_event_type event)
 	E2S(NEW_STA);
 	E2S(EAPOL_RX);
 	E2S(SIGNAL_CHANGE);
+	E2S(INTERFACE_MAC_CHANGED);
 	E2S(INTERFACE_ENABLED);
 	E2S(INTERFACE_DISABLED);
 	E2S(CHANNEL_LIST_CHANGED);
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index dfa11834c..13f5959b6 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -951,7 +951,7 @@  nl80211_find_drv(struct nl80211_global *global, int idx, u8 *buf, size_t len,
 
 
 static void nl80211_refresh_mac(struct wpa_driver_nl80211_data *drv,
-				int ifindex)
+				int ifindex, Boolean notify)
 {
 	struct i802_bss *bss;
 	u8 addr[ETH_ALEN];
@@ -970,6 +970,11 @@  static void nl80211_refresh_mac(struct wpa_driver_nl80211_data *drv,
 			   ifindex, bss->ifname,
 			   MAC2STR(bss->addr), MAC2STR(addr));
 		os_memcpy(bss->addr, addr, ETH_ALEN);
+		if (notify) {
+			wpa_supplicant_event(drv->ctx,
+					     EVENT_INTERFACE_MAC_CHANGED,
+					     NULL);
+		}
 	}
 }
 
@@ -1041,11 +1046,11 @@  static void wpa_driver_nl80211_event_rtm_newlink(void *ctx,
 		namebuf[0] = '\0';
 		if (if_indextoname(ifi->ifi_index, namebuf) &&
 		    linux_iface_up(drv->global->ioctl_sock, namebuf) > 0) {
-			/* Re-read MAC address as it may have changed */
-			nl80211_refresh_mac(drv, ifi->ifi_index);
 			wpa_printf(MSG_DEBUG, "nl80211: Ignore interface down "
 				   "event since interface %s is up", namebuf);
 			drv->ignore_if_down_event = 0;
+			/* Re-read MAC address as it may have changed */
+			nl80211_refresh_mac(drv, ifi->ifi_index, TRUE);
 			return;
 		}
 		wpa_printf(MSG_DEBUG, "nl80211: Interface down (%s/%s)",
@@ -1091,7 +1096,7 @@  static void wpa_driver_nl80211_event_rtm_newlink(void *ctx,
 				   "removed", drv->first_bss->ifname);
 		} else {
 			/* Re-read MAC address as it may have changed */
-			nl80211_refresh_mac(drv, ifi->ifi_index);
+			nl80211_refresh_mac(drv, ifi->ifi_index, FALSE);
 
 			wpa_printf(MSG_DEBUG, "nl80211: Interface up");
 			drv->if_disabled = 0;
diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 4f3604358..e8cbac548 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -4366,6 +4366,9 @@  void wpa_supplicant_event(void *ctx, enum wpa_event_type event,
 			data->signal_change.current_noise,
 			data->signal_change.current_txrate);
 		break;
+	case EVENT_INTERFACE_MAC_CHANGED:
+		wpa_supplicant_update_mac_addr(wpa_s);
+		break;
 	case EVENT_INTERFACE_ENABLED:
 		wpa_dbg(wpa_s, MSG_DEBUG, "Interface was enabled");
 		if (wpa_s->wpa_state == WPA_INTERFACE_DISABLED) {