[PATCH] service: Prevent auto connection during passphrase request

[PATCH] service: Prevent auto connection during passphrase request

[VAUTRIN Emmanuel (Canal Plus Prestataire)]

Avoid concurrent associations between a user connection to a secure
service, blocked by an agent passphrase input request, and a starting
auto connection on a different service of the same interface.

This issue happens when the end-user wants to connect to a new secure
Wifi network (W2) via an interface where another Wifi network (W1) is
already connected. When doing that, the following steps happen:
1. Disconnection of current network (W1).
2. User-connection to W2, held by the passphrase request.
3. Auto-connection to a known available Wifi network (W1) succeeding.
4. User-connection to W2, reactivated by the passphrase response,
failing, as another Wifi (W1) is already connected.
Wifi 2 will never be able, in this configuration, to connect.
---
 src/service.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/src/service.c b/src/service.c
index 20917a8923a4..f6a4e53fe262 100644
--- a/src/service.c
+++ b/src/service.c
@@ -49,6 +49,7 @@ static DBusConnection *connection = NULL;
 
 static GList *service_list = NULL;
 static GHashTable *service_hash = NULL;
+static GHashTable *passphrase_requested = NULL;
 static GSList *counter_list = NULL;
 static unsigned int autoconnect_id = 0;
 static unsigned int vpn_autoconnect_id = 0;
@@ -4209,6 +4210,7 @@ static bool auto_connect_service(GList *services,
 	bool ignore[MAX_CONNMAN_SERVICE_TYPES] = { };
 	bool autoconnecting = false;
 	GList *list;
+	int index;
 
 	DBG("preferred %d sessions %d reason %s", preferred, active_count,
 		reason2string(reason));
@@ -4230,6 +4232,11 @@ static bool auto_connect_service(GList *services,
 			continue;
 		}
 
+		index = __connman_service_get_index(service);
+		if (g_hash_table_lookup(passphrase_requested,
+					GINT_TO_POINTER(index)))
+			return true;
+
 		if (service->pending ||
 				is_connecting(service->state) ||
 				is_connected(service->state)) {
@@ -5780,6 +5787,7 @@ static void request_input_cb(struct connman_service *service,
 	struct connman_device *device;
 	const char *security;
 	int err = 0;
+	int index;
 
 	DBG("RequestInput return, %p", service);
 
@@ -5842,6 +5850,10 @@ static void request_input_cb(struct connman_service *service,
 		err = __connman_service_set_passphrase(service, passphrase);
 
  done:
+	index = __connman_service_get_index(service);
+	g_hash_table_remove(passphrase_requested,
+				GINT_TO_POINTER(index));
+
 	if (err >= 0) {
 		/* We forget any previous error. */
 		set_error(service, CONNMAN_SERVICE_ERROR_UNKNOWN);
@@ -6693,6 +6705,7 @@ static int service_connect(struct connman_service *service)
 int __connman_service_connect(struct connman_service *service,
 			enum connman_service_connect_reason reason)
 {
+	int index;
 	int err;
 
 	DBG("service %p state %s connect reason %s -> %s",
@@ -6779,6 +6792,13 @@ int __connman_service_connect(struct connman_service *service,
 			if (service->hidden && err != -EINPROGRESS)
 				service->pending = pending;
 
+			if (err == -EINPROGRESS) {
+				index = __connman_service_get_index(service);
+				g_hash_table_replace(passphrase_requested,
+						GINT_TO_POINTER(index),
+						GINT_TO_POINTER(true));
+			}
+
 			return err;
 		}
 	}
@@ -7750,6 +7770,8 @@ int __connman_service_init(void)
 	service_hash = g_hash_table_new_full(g_str_hash, g_str_equal,
 							NULL, service_free);
 
+	passphrase_requested = g_hash_table_new(g_direct_hash, g_direct_equal);
+
 	services_notify = g_new0(struct _services_notify, 1);
 	services_notify->remove = g_hash_table_new_full(g_str_hash,
 			g_str_equal, g_free, NULL);
@@ -7782,6 +7804,9 @@ void __connman_service_cleanup(void)
 	g_hash_table_destroy(service_hash);
 	service_hash = NULL;
 
+	g_hash_table_destroy(passphrase_requested);
+	passphrase_requested = NULL;
+
 	g_slist_free(counter_list);
 	counter_list = NULL;
 
-- 
2.25.1
-------------------------------------------------------------------------
Hi Daniel,

A reminder of the former thread:

> Ah I see you dropped the file descriptor and now you just store a
> boolean. In this case you could just add a boolean variable called
> passphrase_requested to 'struct service' and you could drop the hash
> table completely. So this gets
> 
>       service->passphrase_requested = true;
>
> and in auto_connect_servcie() a simple
> 
>        if (service->passphrase_requested)
>               return true;
>
> is enough.

These information are at interface level,
and shall be shared between all related services.
I doubt setting those at service side will solve the problem.


Best Regards,

Emmanuel

RE: [PATCH] service: Prevent auto connection during passphrase request

[VAUTRIN Emmanuel (Canal Plus Prestataire)]

Hi,

Was there any progress on this subject? Does it still require some clarification?


Best Regards,

Emmanuel

Re: [PATCH] service: Prevent auto connection during passphrase request

[Daniel Wagner]

Hi Emmanuel,

On Wed, Aug 25, 2021 at 12:27:50PM +0000, VAUTRIN Emmanuel (Canal Plus Prestataire) wrote:
> Was there any progress on this subject? Does it still require some clarification?

No all good. Thanks for extending the commit message. This will be handy
to figure out what it tries to fix if there is a problem with this
change.

Patch applied.
Thanks,
Daniel

RE: [PATCH] service: Prevent auto connection during passphrase request

[VAUTRIN Emmanuel (Canal Plus Prestataire)]

>  No all good. Thanks for extending the commit message. This will be handy
> to figure out what it tries to fix if there is a problem with this
> change.
> 
> Patch applied.
Great Daniel, thank you.

Emmanuel