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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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


_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap



[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux