Re: [PATCH] wifi: Handle invalid-key case on WPA-SAE authentication failure

[Ariel D'Alessandro <ariel.dalessandro@collabora.com>]

+cc Daniel, in case you have any possible feedback :-)

On 9/14/22 15:57, Ariel D'Alessandro wrote:
> Hi all, John,
> 
> This patch is a follow up of my question e-mail with subject "No
> password prompt after wrong entry".
> 
> There's currently a difference on the state transitions between WPA2-PSK
> and WPA3-SAE, which makes the latter not to prompt for a new password
> after an invalid key has been sent and rejected. Note that on the former
> case (WPA2-PSK) that works as expected and the client agent asks for a
> new password if the key was invalid.
> 
> The issue comes from the state transitions on each case.
> 
> For the WPA2-PSK wrong-key case, connman goes through states:
> * G_SUPPLICANT_STATE_AUTHENTICATING
> * G_SUPPLICANT_STATE_4WAY_HANDSHAKE
> * G_SUPPLICANT_STATE_DISCONNECTED
> 
> So, the invalid-key error is handled and reported here:
> 
> https://git.kernel.org/pub/scm/network/connman/connman.git/tree/plugins/wifi.c#n2526
> 
> However, for the WPA3-SAE wrong-key case, connman goes through states:
> * G_SUPPLICANT_STATE_AUTHENTICATING
> * G_SUPPLICANT_STATE_DISCONNECTED
> 
> So, the invalid-key error never gets reported. Instead, connect-failed
> is reported by connman, which makes the client agent never prompt for a
> new password.
> 
> Any feedback is welcome, specially if the proposed solution should be
> implemented in a different way
> 
> Thanks in advance :-)
> Ariel D'Alessandro
> --
> Collabora Ltd.
> https://www.collabora.com/
> 
> On 9/14/22 15:46, Ariel D'Alessandro wrote:
>> On WPA3-SAE authentication, wpa_supplicant goes directly from
>> authenticating to disconnected state if the key was invalid.
>>
>> The above is currently not handled and the `connect-failed` error is
>> reported on such cases. In order to make the client agent prompt for a
>> new password, we need to handle this transition and report the
>> `invalid-key` error.
>>
>> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
>> ---
>>  plugins/wifi.c | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>
>> diff --git a/plugins/wifi.c b/plugins/wifi.c
>> index 2a933708..ed7437f5 100644
>> --- a/plugins/wifi.c
>> +++ b/plugins/wifi.c
>> @@ -2528,6 +2528,25 @@ static bool handle_4way_handshake_failure(GSupplicantInterface *interface,
>>  	return false;
>>  }
>>  
>> +static bool handle_sae_authentication_failure(struct connman_network *network,
>> +					      struct wifi_data *wifi)
>> +{
>> +	struct wifi_network *network_data = connman_network_get_data(network);
>> +
>> +	if (!(network_data->keymgmt & G_SUPPLICANT_KEYMGMT_SAE))
>> +		return false;
>> +
>> +	if (wifi->state != G_SUPPLICANT_STATE_AUTHENTICATING)
>> +		return false;
>> +
>> +	if (wifi->connected)
>> +		return false;
>> +
>> +	connman_network_set_error(network, CONNMAN_NETWORK_ERROR_INVALID_KEY);
>> +
>> +	return true;
>> +}
>> +
>>  static void interface_state(GSupplicantInterface *interface)
>>  {
>>  	struct connman_network *network;
>> @@ -2625,6 +2644,13 @@ static void interface_state(GSupplicantInterface *interface)
>>  						network, wifi))
>>  			break;
>>  
>> +		/*
>> +		 * On WPA3-SAE authentication, wpa_supplicant goes directly from
>> +		 * authenticating to disconnected state if the key was invalid.
>> +		 */
>> +		if (handle_sae_authentication_failure(network, wifi))
>> +			break;
>> +
>>  		/* See table 8-36 Reason codes in IEEE Std 802.11 */
>>  		switch (wifi->disconnect_code) {
>>  		case 6: /* Class 2 frame received from nonauthenticated STA */
>