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

[Jouni Malinen]

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.

> 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?

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

-- 
Jouni Malinen                                            PGP id EFC895FA