[PATCH 1/1] dbus: add a new property SAEConfirmMismatch

[xinpeng wang]

On 2024/12/31 00:47, Jouni Malinen wrote:
> On Wed, Dec 25, 2024 at 07:23:44PM +0800, xinpeng wang wrote:
>> Add a new dbus property SAEConfirmMismatch to notify the desktop
>> that a password dialog needs to pop up for the user to enter the
>> correct password
> I'm still unhappy about this commit message implying that SAE Confirm
> mismatch would always need user to be asked to enter a new password.
> That would be inappropriate behavior to do unconditionally since there
> is no protection against attackers, i.e., this could easily be forced to
> happen with a rogue AP and it is not really good to imply that this
> indication is in any way robust indication of the currently configured
> password being wrong.
Thank you for your reply. I will submit a new patch and explain in the
commit message that the newly added SAEConfirmMismatch only indicates
that the second round of confirmation failed during the SAE authentication
process. The user will handle it flexibly based on this and consider the
possibility of being attacked by rogue APs.

>> diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c
>> @@ -4191,6 +4194,13 @@ static const struct wpa_dbus_property_desc wpas_dbus_interface_properties[] = {
>> +#if defined(CONFIG_SAE) && defined(CONFIG_SME)
>> +	{ "SAEConfirmMismatch", WPAS_DBUS_NEW_IFACE_INTERFACE, "b",
>> +	  wpas_dbus_getter_sae_confirm_mismatch,
>> +	  NULL,
>> +	  NULL
>> +	},
>> +#endif /* CONFIG_SME && CONFIG_SAE */
> DBus interface changes need to be documented in doc/dbus.doxygen in
> addition to the implementation changes.
>
> Could you please describe why using a property instead of a signal is
> the best way of adding this?
The program that processes this signal is NetworkManager. I found that
handling property changed in nm is simpler than adding a new signal,
so I added it as an property.
Actually, the signal is more reasonable than the property, I will modify
it to signal.

>> diff --git a/wpa_supplicant/notify.c b/wpa_supplicant/notify.c
>> +void wpas_notify_sae_confirm_mismatch(struct wpa_supplicant *wpa_s)
>> +{
>> +	if (wpa_s->p2p_mgmt)
>> +		return;
>> +
>> +	/* notify the new DBus API */
>> +	wpas_dbus_signal_prop_changed(wpa_s, WPAS_DBUS_PROP_SAE_CONFIRM_MISMATCH);
>> +}
> What about property changes from TRUE to FALSE? This design would only
> cover the change to TRUE and no update after that when SAE state is
> cleared. I.e., no indication to the upper layer component on all the
> property changes.
>
>> diff --git a/wpa_supplicant/sme.c b/wpa_supplicant/sme.c
>> @@ -1820,6 +1820,17 @@ static int sme_sae_auth(struct wpa_supplicant *wpa_s, u16 auth_transaction,
>>   
>> +	if (auth_transaction == 2 &&
>> +	    status_code == WLAN_REASON_UNSPECIFIED) {
>> +		/* Some APs will only send confirmation after receiving the correct confirmation
>> +		   sent by STA, otherwise they will send status_code=WLAN_REASON_UNSPECIFIED.
>> +		   In order to allow the desktop to pop up the password dialog in this case,here
>> +		   also notify SAEConfirmMismatch */
> That comment is implying behavior that would result in very bad user
> experience since there are no guarantees of this being caused by
> incorrect password. In addition, WLAN_REASON_UNSPECIFIC is an incorrect
> thing to compare against status codes; it is a reason code, not a status
> code.. A status code could be WLAN_STATUS_UNSPECIFIED_FAILURE, but that
> would also be wrong here since the failure to verify SAE Confirm is
> indicated with the status code CHALLENGE_FAILURE (i.e.,
> WLAN_STATUS_CHALLENGE_FAIL).
>
I use the Andriod mobile phone to open the hotspot of WPA3 mode and
connect it.When entering the error password,the mobile phone will not
send confirm, but the status=WLAN_STATUS_UNSPECIFIED_FAILURE in the message.
In order to allow the desktop program to perceive the password error in
this case, I added this code.
If there is a disadvantage of sending SAEConfirmMismatch, I will delete
this code. Or do you think there are other better ways to deal with this
situation?
Thank you again for your reply