diff mbox series

[2/2] eapol: warn rather than reject invalid PMKID (for EAP)

Message ID 20230404203823.384260-2-prestwoj@gmail.com (mailing list archive)
State New
Headers show
Series [1/2] handshake: include additional sha256 AKMs for PMKID generation | expand

Commit Message

James Prestwood April 4, 2023, 8:38 p.m. UTC
A recent hostapd change b6d3fd05e3 modified the PMKID derivation
which breaks EAPoL if the FT-8021x AKM is used and the AP sends
the PMKID KDE in message 1. This is because if the PMKID does not
validate it kicks off EAP again to renegotiate a PMK, but ultimately
the PMKID generation doesn't change so we end up in a loop until the
handshake timeout.

The validation of the PMKID isn't really required since IWD doesn't
support PMKSA, but we do it anyways if the KDE is included (why not
right?). But now with this interoperability issue we have to work
around APs incorrectly deriving the PMKID since its been in hostapd
for quite some time and a guarantee there are APs in production with
this issue.

For FT-PSK there is no changes required since IWD already ignores
a mismatch (see comment about zero/random PMKID). For FT-8021x
IWD will now first check if EAP has been exchanged and in that
case ignore the mismatch and print a warning.
---
 src/eapol.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Denis Kenzior April 9, 2023, 5:22 p.m. UTC | #1
Hi James,

On 4/4/23 15:38, James Prestwood wrote:
> A recent hostapd change b6d3fd05e3 modified the PMKID derivation
> which breaks EAPoL if the FT-8021x AKM is used and the AP sends
> the PMKID KDE in message 1. This is because if the PMKID does not
> validate it kicks off EAP again to renegotiate a PMK, but ultimately
> the PMKID generation doesn't change so we end up in a loop until the
> handshake timeout.
> 
> The validation of the PMKID isn't really required since IWD doesn't
> support PMKSA, but we do it anyways if the KDE is included (why not
> right?). But now with this interoperability issue we have to work
> around APs incorrectly deriving the PMKID since its been in hostapd
> for quite some time and a guarantee there are APs in production with
> this issue.
> 
> For FT-PSK there is no changes required since IWD already ignores
> a mismatch (see comment about zero/random PMKID). For FT-8021x
> IWD will now first check if EAP has been exchanged and in that
> case ignore the mismatch and print a warning.
> ---
>   src/eapol.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/eapol.c b/src/eapol.c
> index 3d7d33e0..43f65b85 100644
> --- a/src/eapol.c
> +++ b/src/eapol.c
> @@ -1237,11 +1237,17 @@ static void eapol_handle_ptk_1_of_4(struct eapol_sm *sm,
>   			/*
>   			 * If the AP has a different PMKSA from ours and we
>   			 * have means to create a new PMKSA through EAP then
> -			 * try that, otherwise give up.
> +			 * try that, otherwise give up. If EAP has already been
> +			 * exchanged its likely the AP is using an outdated
> +			 * derivation, in this case continue with a warning.
>   			 */
>   			if (sm->eap) {
> -				__send_eapol_start(sm, unencrypted);
> -				return;
> +				if (!sm->eap_exchanged) {
> +					__send_eapol_start(sm, unencrypted);
> +					return;
> +				}
> +
> +				l_warn("AP may be using old PMKID derivation!");

Ugh.  Why not just detect that this is an FT-8021X AKM and compare both the 
'proper' and the 'compatibility' PMKIDs?

>   			}
>   
>   			/*

Regards,
-Denis
diff mbox series

Patch

diff --git a/src/eapol.c b/src/eapol.c
index 3d7d33e0..43f65b85 100644
--- a/src/eapol.c
+++ b/src/eapol.c
@@ -1237,11 +1237,17 @@  static void eapol_handle_ptk_1_of_4(struct eapol_sm *sm,
 			/*
 			 * If the AP has a different PMKSA from ours and we
 			 * have means to create a new PMKSA through EAP then
-			 * try that, otherwise give up.
+			 * try that, otherwise give up. If EAP has already been
+			 * exchanged its likely the AP is using an outdated
+			 * derivation, in this case continue with a warning.
 			 */
 			if (sm->eap) {
-				__send_eapol_start(sm, unencrypted);
-				return;
+				if (!sm->eap_exchanged) {
+					__send_eapol_start(sm, unencrypted);
+					return;
+				}
+
+				l_warn("AP may be using old PMKID derivation!");
 			}
 
 			/*