[PATCH] service: Fix native connection with wrong passphrase

[PATCH] service: Fix native connection with wrong passphrase

[Emmanuel VAUTRIN]

When a native connection fails with a wrong passphrase, the user still
needs to be informed, via the agent.
However, in this case, the associated service is automatically
disconnected, canceling the pending agent requests.
The invalid key error shall be reported before this cancellation.

Commit b9a0a039ccc5 ("service: Report errors to user in native mode")
---
 src/service.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/service.c b/src/service.c
index 9141b85dbe9c..202816ae8980 100644
--- a/src/service.c
+++ b/src/service.c
@@ -6023,6 +6023,11 @@ static int service_indicate_state(struct connman_service *service)
 		break;
 
 	case CONNMAN_SERVICE_STATE_IDLE:
+		if (old_state == CONNMAN_SERVICE_STATE_FAILURE && 
+	service->connect_reason == CONNMAN_SERVICE_CONNECT_REASON_NATIVE &&
+	service->error == CONNMAN_SERVICE_ERROR_INVALID_KEY)
+		report_error_cb(service, false, NULL);
+
 		if (old_state != CONNMAN_SERVICE_STATE_DISCONNECT)
 			__connman_service_disconnect(service);
 
-- 
2.25.1

Re: [PATCH] service: Fix native connection with wrong passphrase

[Daniel Wagner]

Hi Emmanuel,

On Thu, Dec 16, 2021 at 11:22:49AM +0100, Emmanuel VAUTRIN wrote:
> When a native connection fails with a wrong passphrase, the user still
> needs to be informed, via the agent.
> However, in this case, the associated service is automatically
> disconnected, canceling the pending agent requests.
> The invalid key error shall be reported before this cancellation.
> 
> Commit b9a0a039ccc5 ("service: Report errors to user in native mode")
> ---
>  src/service.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/service.c b/src/service.c
> index 9141b85dbe9c..202816ae8980 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -6023,6 +6023,11 @@ static int service_indicate_state(struct connman_service *service)
>  		break;
>  
>  	case CONNMAN_SERVICE_STATE_IDLE:
> +		if (old_state == CONNMAN_SERVICE_STATE_FAILURE && 
> +	service->connect_reason == CONNMAN_SERVICE_CONNECT_REASON_NATIVE &&
> +	service->error == CONNMAN_SERVICE_ERROR_INVALID_KEY)
> +		report_error_cb(service, false, NULL);

There is whitespace damaged and a pretty idention. I was about to fix
this, but stopped because the 'report_error_cb()' doesn't make sense
with the commit message. If you want to properly report you need

			connman_agent_report_error(service, service->path,
						error2string(service->error),
						report_error_cb,
						get_dbus_sender(service),
						NULL);

IMO. report_error_cb() doesn't do any agent stuff.

Daniel

Re: [PATCH] service: Fix native connection with wrong passphrase

[Emmanuel Vautrin]

Hi Daniel,

> There is whitespace damaged and a pretty idention.
Indeed, you are right. I am trying to follow your advice and using
send-email now, via gmail.
When displaying my original message via the gmail client, the
formatting seems to be
the expected one, so it seems not to be related to git send-email
command itself.
Unfortunately, I have not found how to prevent gmail post-formatting the mail,
and I doubt it can be configured. Do you have any idea how I can move
on this subject?

> I was about to fix this, but stopped because the 'report_error_cb()' doesn't make sense
> with the commit message. If you want to properly report you need
>
>                        connman_agent_report_error(service, service->path,
>                                                error2string(service->error),
>                                                report_error_cb,
>                                                get_dbus_sender(service),
>                                                NULL);

> IMO. report_error_cb() doesn't do any agent stuff.
In fact, the connman_agent_report_error is well called, but the report_error_cb
is not. As my solution is probably far from a clean one, the best is
to explain the issue.

I am focusing on 2 close cases, connection to a secured network via iwd with
rejected (wrong length / content) passphrases.

**Connection with a wrong passphrase length **
The error is directly managed in request_input_cb, due to a failing call to
__connman_service_check_passphrase (length < 8 || length > 63):
#0  connman_agent_report_error at ../git/src/agent.c:437
#1  service_indicate_state at ../git/src/service.c:6139
#2  __connman_service_ipconfig_indicate_state at ../git/src/service.c:6492
#3  __connman_service_indicate_error at ../git/src/service.c:6183
#4  request_input_cb at ../git/src/service.c:5875
#5  request_input_passphrase_reply at ../git/src/agent-connman.c:204
#6  agent_finalize_pending at ../git/src/agent.c:121
#7  agent_receive_message at ../git/src/agent.c:208

then the reply pending is done, as expected:
#0  reply_pending at ../git/src/service.c:3963
#1 service_complete at ../git/src/service.c:3980
#2 report_error_cb at ../git/src/service.c:5752
#3 report_error_reply at ../git/src/agent.c:369
#4 agent_finalize_pending at ../git/src/agent.c:121
#5 agent_receive_message at ../git/src/agent.c:208

**Connection with a wrong passphrase**
The error is managed in native cm_network_connect_cb:
#0 connman_agent_report_error  at ../git/src/agent.c:437
#1 service_indicate_state  at ../git/src/service.c:6139
#2 __connman_service_ipconfig_indicate_state  at ../git/src/service.c:6492
#3 __connman_service_indicate_error at ../git/src/service.c:6183
#4 set_invalid_key_error at ../git/src/network.c:1582
#5 connman_network_set_error at ../git/src/network.c:1655
#6 in cm_network_connect_cb at ../git/plugins/iwd.c:245
#7 in method_call_reply at ../git/gdbus/client.c:831

But, unfortunately, the native __connman_network_disconnect is called,
canceling the request, what prevents the report_error_cb to be called:
#0  send_cancel_request at ../git/src/agent.c:166
#1  connman_agent_cancel at ../git/src/agent.c:547
#2  __connman_service_disconnect at ../git/src/service.c:6817
#3  service_indicate_state at ../git/src/service.c:6022
#4  __connman_service_ipconfig_indicate_state at ../git/src/service.c:6492
#5  set_disconnected at ../git/src/network.c:1044
#6  __connman_network_disconnect at ../git/src/network.c:1848
#7  connman_network_set_error at ../git/src/network.c:1665
#8  cm_network_connect_cb at ../git/plugins/iwd.c:245
#9  method_call_reply at ../git/gdbus/client.c:831

I hope having clarified the problem.


Best Regards,

Emmanuel