Send connman mailing list submissions to
        [email protected]

To subscribe or unsubscribe via the World Wide Web, visit
        https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
        [email protected]

You can reach the person managing the list at
        [email protected]

When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."


Today's Topics:

   1. [PATCH 00/10] Improve handling of VPN agent return types
      (Jussi Laakkonen)
   2. [PATCH 01/10] vpn-agent: Implement generic D-Bus error
      checker for plugins. (Jussi Laakkonen)
   3. [PATCH 02/10] vpn-provider: React to different error types in
      connect_cb() (Jussi Laakkonen)
   4. [PATCH 03/10] l2tp: Use vpn-agent.c error processing for VPN
      agent errors. (Jussi Laakkonen)
   5. [PATCH 04/10] openconnect: Use vpn-agent.c error processing
      for VPN agent errors. (Jussi Laakkonen)
   6. [PATCH 05/10] pptp: Use vpn-agent.c error processing for VPN
      agent errors. (Jussi Laakkonen)
   7. [PATCH 06/10] vpnc: Use vpn-agent.c error processing for VPN
      agent errors. (Jussi Laakkonen)


----------------------------------------------------------------------

Message: 1
Date: Mon, 24 Jun 2019 17:37:43 +0300
From: Jussi Laakkonen <[email protected]>
To: [email protected]
Subject: [PATCH 00/10] Improve handling of VPN agent return types
Message-ID: <[email protected]>

The different returns from VPN agent should be handled accordingly in VPN
plugins as well as in connmand. Otherwise there will be a race between each
VPN agent supporting VPN type as canceling or timeouting the VPN agent dialog
would not unset the autoconnect value (to false) and change is not propagated to
service file written to disk. This will manifest itself after the next restart
of connmand as continuous connect-disconnect-connect loop between the VPNs that
were attempted to connect but their VPN agent dialog was canceled.

This set of patches enables processing of the different VPN agent return types
and ensures that autoconnect value changes are done internally and written to
disk. Improved processing of VPN agent return is used in the VPN plugins that
utilize it (L2TP, OpenConnect, PPTP and VPNC, OpenVPN is excluded) and a proper
reply is sent back to caller.

Each aforementioned plugin should not have own implementation of error checking
and thus, a generic one is implemented in vpn-agent.c. Processing of the error
is done using the callback of the VPN plugin, which eventually is
vpn-provider.c:connect_cb() that is also amended to support the different error
types. If there is no error returned from VPN agent normal operation is resumed.

The returned error is then passed to caller as appropriate D-Bus error message.
The caller (plugins/vpn.c) should be able to disable VPN autoconnect if the user
has cancelled the VPN agent dialog. For this the new error type
OperationCanceled (ECANCELED) is implemented and setters for autoconnect value
are created into provider.c and service.c.

Jussi Laakkonen (10):
  vpn-agent: Implement generic D-Bus error checker for plugins.
  vpn-provider: React to different error types in connect_cb()
  l2tp: Use vpn-agent.c error processing for VPN agent errors.
  openconnect: Use vpn-agent.c error processing for VPN agent errors.
  pptp: Use vpn-agent.c error processing for VPN agent errors.
  vpnc: Use vpn-agent.c error processing for VPN agent errors.
  service: Implement function to set service autoconnect value
  provider: Implement function to set VPN service autoconnect value
  error: Implement OperationCanceled error type
  vpn: Handle OperationCanceled D-Bus message reply

 include/provider.h        |  2 ++
 include/service.h         |  2 ++
 plugins/vpn.c             | 14 +++++++++++-
 src/connman.h             |  1 +
 src/error.c               |  8 +++++++
 src/provider.c            | 11 +++++++++
 src/service.c             | 12 ++++++++++
 vpn/plugins/l2tp.c        | 18 ++++++++++++---
 vpn/plugins/openconnect.c | 11 +++++++--
 vpn/plugins/pptp.c        | 18 ++++++++++++---
 vpn/plugins/vpnc.c        | 40 +++++++-------------------------
 vpn/vpn-agent.c           | 48 +++++++++++++++++++++++++++++++++++++++
 vpn/vpn-agent.h           |  4 ++++
 vpn/vpn-provider.c        | 46 +++++++++++++++++++++++++++++++++++--
 14 files changed, 192 insertions(+), 43 deletions(-)

-- 
2.20.1



------------------------------

Message: 2
Date: Mon, 24 Jun 2019 17:37:44 +0300
From: Jussi Laakkonen <[email protected]>
To: [email protected]
Subject: [PATCH 01/10] vpn-agent: Implement generic D-Bus error
        checker for plugins.
Message-ID: <[email protected]>

It is not feasible for every VPN plugin to check the errors in D-Bus
reply sent by VPN agent in their own way. This kind of general use makes
reacting to the canceled, timed out or no response replies sent by VPN
agent more common and less error prone.

The check function (vpn_agent_check_and_process_reply_error()) takes in
the reply, callback to provider connect function and the pending D-Bus
message, that will be utilized by the callback
(vpn-provider.c:connect_cb()). This way proper notifications are passed
forwards and appropriate reply is sent to the caller of the VPN plugin
connection request (connmand). By sending the reply the caller
(connmand) can then disable autoconnection from the VPN to avoid
re-connection in case the user canceled the VPN agent dialog.

The errors that are reacted to:
 - net.connman.vpn.Agent.Error.Canceled -> ECANCELED
 - org.freedesktop.DBus.Error.Timeout -> ETIMEDOUT
 - org.freedesktop.DBus.Error.NoReply -> ENOMSG
 - any other error from VPN agent is EACCES

Error in VPN agent or canceling the VPN agent should not be an error in
the VPN provider and, therefore, set the provider state to idle. Also,
if task was already running stop it to avoid leaving VPN processes
running.
---
 vpn/vpn-agent.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 vpn/vpn-agent.h |  4 ++++
 2 files changed, 52 insertions(+)

diff --git a/vpn/vpn-agent.c b/vpn/vpn-agent.c
index b0b582b7..82771b92 100644
--- a/vpn/vpn-agent.c
+++ b/vpn/vpn-agent.c
@@ -30,6 +30,8 @@
 #include <gdbus.h>
 #include <connman/log.h>
 #include <connman/agent.h>
+#include <connman/vpn-dbus.h>
+#include <connman/task.h>
 #include <vpn/vpn-provider.h>
 
 #include "vpn-agent.h"
@@ -143,3 +145,49 @@ void vpn_agent_append_user_info(DBusMessageIter *iter,
                                request_input_append_user_info,
                                &data);
 }
+
+int vpn_agent_check_and_process_reply_error(DBusMessage *reply,
+                               struct vpn_provider *provider,
+                               struct connman_task *task,
+                               vpn_provider_connect_cb_t cb, void *user_data)
+{
+       DBusError error;
+       int err;
+
+       if (!reply || !provider)
+               return EINVAL;
+
+       dbus_error_init(&error);
+
+       if (!dbus_set_error_from_message(&error, reply))
+               return 0;
+
+       DBG("D-Bus error: %s", error.name);
+
+       if (!g_strcmp0(error.name, VPN_AGENT_INTERFACE ".Error.Canceled"))
+               err = ECANCELED;
+       else if (!g_strcmp0(error.name, "org.freedesktop.DBus.Error.Timeout"))
+               err = ETIMEDOUT;
+       else if (!g_strcmp0(error.name, "org.freedesktop.DBus.Error.NoReply"))
+               err = ENOMSG;
+       else
+               err = EACCES;
+
+       dbus_error_free(&error);
+
+       DBG("Error: %d/%s", err, strerror(err));
+
+       if (cb)
+               cb(provider, user_data, err);
+
+       if (task)
+               connman_task_stop(task);
+
+       /*
+        * VPN agent dialog cancel, timeout, broken connection should set the
+        * VPN back to idle state
+        */
+       vpn_provider_set_state(provider, VPN_PROVIDER_STATE_IDLE);
+
+       return err;
+}
diff --git a/vpn/vpn-agent.h b/vpn/vpn-agent.h
index c7328d7f..be7f9dd9 100644
--- a/vpn/vpn-agent.h
+++ b/vpn/vpn-agent.h
@@ -38,6 +38,10 @@ bool vpn_agent_check_reply_has_dict(DBusMessage *reply);
 void vpn_agent_append_user_info(DBusMessageIter *iter,
                                struct vpn_provider *provider,
                                const char *username_str);
+int vpn_agent_check_and_process_reply_error(DBusMessage *reply,
+                               struct vpn_provider *provider,
+                               struct connman_task *task,
+                               vpn_provider_connect_cb_t cb, void *user_data);
 
 #ifdef __cplusplus
 }
-- 
2.20.1



------------------------------

Message: 3
Date: Mon, 24 Jun 2019 17:37:45 +0300
From: Jussi Laakkonen <[email protected]>
To: [email protected]
Subject: [PATCH 02/10] vpn-provider: React to different error types in
        connect_cb()
Message-ID: <[email protected]>

The connection callback (connect_cb()) can be called via vpn-agent.c to
indicate that VPN agent dialog had an error or was cancelled in addition
to calling the function via VPN plugin. Because of this the different
error types should be handled properly.

Errors:
 - EACCES is an authentication error: VPN_PROVIDER_ERROR_AUTH_FAILED.
 - ENOMSG and ETIMEDOUT are system reported errors and then the agent
   request needs to be canceled and error set unknown error.
 - ECANCELED is reported when user canceled VPN agent dialog, treat this
   as same as ECONNABORTED as the VPN may have been initialized already.
 - ECONNABORTED is set when connect_cb() is called via VPN plugin. To
   ensure that proper disconnect -> idle cycle is done both driver and
   provider are set to disconnect state and eventually killed and put to
   idle state if the provider was 1) being connected or 2) already
   connected.
 - In other cases the VPN provider is set to failure state and connect
   error is indicated.
---
 vpn/vpn-provider.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index 053c1a9a..29d93a41 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -1089,9 +1089,51 @@ static void connect_cb(struct vpn_provider *provider, 
void *user_data,
                if (reply)
                        g_dbus_send_message(connection, reply);
 
-               vpn_provider_indicate_error(provider,
+               switch (error) {
+               case EACCES:
+                       vpn_provider_indicate_error(provider,
+                                               VPN_PROVIDER_ERROR_AUTH_FAILED);
+                       break;
+               case ENOMSG:
+                       /* fall through */
+               case ETIMEDOUT:
+                       /* No reply or timed out -> cancel the agent request */
+                       connman_agent_cancel(provider);
+                       vpn_provider_indicate_error(provider,
+                                               VPN_PROVIDER_ERROR_UNKNOWN);
+                       break;
+               case ECANCELED:
+                       /* fall through */
+               case ECONNABORTED:
+                       /*
+                        * This can be called in other situations than when
+                        * VPN agent error checker is called. In such case
+                        * react to both ECONNABORTED and ECANCELED as if the
+                        * connection was called to terminate and do full
+                        * disconnect -> idle cycle when being connected or
+                        * ready. Setting the state also using the driver
+                        * callback (vpn_set_state()) ensures that the driver is
+                        * being disconnected as well and eventually the vpn
+                        * process gets killed and vpn_died() is called to make
+                        * the provider back to idle state.
+                        */
+                       if (provider->state == VPN_PROVIDER_STATE_CONNECT ||
+                                               provider->state ==
+                                               VPN_PROVIDER_STATE_READY) {
+                               if (provider->driver->set_state)
+                                       provider->driver->set_state(provider,
+                                               VPN_PROVIDER_STATE_DISCONNECT);
+
+                               vpn_provider_set_state(provider,
+                                               VPN_PROVIDER_STATE_DISCONNECT);
+                       }
+                       break;
+               default:
+                       vpn_provider_indicate_error(provider,
                                        VPN_PROVIDER_ERROR_CONNECT_FAILED);
-               vpn_provider_set_state(provider, VPN_PROVIDER_STATE_FAILURE);
+                       vpn_provider_set_state(provider,
+                                       VPN_PROVIDER_STATE_FAILURE);
+               }
        } else
                g_dbus_send_reply(connection, pending, DBUS_TYPE_INVALID);
 
-- 
2.20.1



------------------------------

Message: 4
Date: Mon, 24 Jun 2019 17:37:46 +0300
From: Jussi Laakkonen <[email protected]>
To: [email protected]
Subject: [PATCH 03/10] l2tp: Use vpn-agent.c error processing for VPN
        agent errors.
Message-ID: <[email protected]>

Use vpn_agent_check_and_process_reply_error() to check and process VPN
agent errors. Clear callback and pending D-Bus message (user_data) if
error was processed to avoid calling the callback twice.
---
 vpn/plugins/l2tp.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/vpn/plugins/l2tp.c b/vpn/plugins/l2tp.c
index 5d83eb88..606feb1b 100644
--- a/vpn/plugins/l2tp.c
+++ b/vpn/plugins/l2tp.c
@@ -491,16 +491,28 @@ struct request_input_reply {
 static void request_input_reply(DBusMessage *reply, void *user_data)
 {
        struct request_input_reply *l2tp_reply = user_data;
+       struct l2tp_private_data *data;
        const char *error = NULL;
        char *username = NULL, *password = NULL;
        char *key;
        DBusMessageIter iter, dict;
+       int err_int;
 
        DBG("provider %p", l2tp_reply->provider);
 
-       if (!reply || dbus_message_get_type(reply) == DBUS_MESSAGE_TYPE_ERROR) {
-               if (reply)
-                       error = dbus_message_get_error_name(reply);
+       if (!reply)
+               goto done;
+
+       data = l2tp_reply->user_data;
+
+       err_int = vpn_agent_check_and_process_reply_error(reply,
+                               l2tp_reply->provider, data->task, data->cb,
+                               data->user_data);
+       if (err_int) {
+               /* Ensure cb is called only once */
+               data->cb = NULL;
+               data->user_data = NULL;
+               error = dbus_message_get_error_name(reply);
                goto done;
        }
 
-- 
2.20.1



------------------------------

Message: 5
Date: Mon, 24 Jun 2019 17:37:47 +0300
From: Jussi Laakkonen <[email protected]>
To: [email protected]
Subject: [PATCH 04/10] openconnect: Use vpn-agent.c error processing
        for VPN agent errors.
Message-ID: <[email protected]>

Use vpn_agent_check_and_process_reply_error() to check and process VPN
agent errors. Clear callback and pending D-Bus message (user_data) if
error was processed to avoid calling the callback twice.
---
 vpn/plugins/openconnect.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/vpn/plugins/openconnect.c b/vpn/plugins/openconnect.c
index 4117471c..5448f647 100644
--- a/vpn/plugins/openconnect.c
+++ b/vpn/plugins/openconnect.c
@@ -311,11 +311,18 @@ static void request_input_cookie_reply(DBusMessage 
*reply, void *user_data)
        char *cookie = NULL, *servercert = NULL, *vpnhost = NULL;
        char *key;
        DBusMessageIter iter, dict;
+       int err_int;
 
        DBG("provider %p", data->provider);
 
-       if (!reply || dbus_message_get_type(reply) == DBUS_MESSAGE_TYPE_ERROR)
-               goto err;
+       err_int = vpn_agent_check_and_process_reply_error(reply, data->provider,
+                               data->task, data->cb, data->user_data);
+       if (err_int) {
+               /* Ensure cb is called only once */
+               data->cb = NULL;
+               data->user_data = NULL;
+               return;
+       }
 
        if (!vpn_agent_check_reply_has_dict(reply))
                goto err;
-- 
2.20.1



------------------------------

Message: 6
Date: Mon, 24 Jun 2019 17:37:48 +0300
From: Jussi Laakkonen <[email protected]>
To: [email protected]
Subject: [PATCH 05/10] pptp: Use vpn-agent.c error processing for VPN
        agent errors.
Message-ID: <[email protected]>

Use vpn_agent_check_and_process_reply_error() to check and process VPN
agent errors. Clear callback and pending D-Bus message (user_data) if
error was processed to avoid calling the callback twice.
---
 vpn/plugins/pptp.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/vpn/plugins/pptp.c b/vpn/plugins/pptp.c
index 3dc93b03..e8d57397 100644
--- a/vpn/plugins/pptp.c
+++ b/vpn/plugins/pptp.c
@@ -282,16 +282,28 @@ struct request_input_reply {
 static void request_input_reply(DBusMessage *reply, void *user_data)
 {
        struct request_input_reply *pptp_reply = user_data;
+       struct pptp_private_data *data;
        const char *error = NULL;
        char *username = NULL, *password = NULL;
        char *key;
        DBusMessageIter iter, dict;
+       int err_int;
 
        DBG("provider %p", pptp_reply->provider);
 
-       if (!reply || dbus_message_get_type(reply) == DBUS_MESSAGE_TYPE_ERROR) {
-               if (reply)
-                       error = dbus_message_get_error_name(reply);
+       if (!reply)
+               goto done;
+
+       data = pptp_reply->user_data;
+
+       err_int = vpn_agent_check_and_process_reply_error(reply,
+                               pptp_reply->provider, data->task, data->cb,
+                               data->user_data);
+       if (err_int) {
+               /* Ensure cb is called only once */
+               data->cb = NULL;
+               data->user_data = NULL;
+               error = dbus_message_get_error_name(reply);
                goto done;
        }
 
-- 
2.20.1



------------------------------

Message: 7
Date: Mon, 24 Jun 2019 17:37:49 +0300
From: Jussi Laakkonen <[email protected]>
To: [email protected]
Subject: [PATCH 06/10] vpnc: Use vpn-agent.c error processing for VPN
        agent errors.
Message-ID: <[email protected]>

Use vpn_agent_check_and_process_reply_error() to check and process VPN
agent errors. Clear callback and pending D-Bus message (user_data) if
error was processed to avoid calling the callback twice.
---
 vpn/plugins/vpnc.c | 40 ++++++++--------------------------------
 1 file changed, 8 insertions(+), 32 deletions(-)

diff --git a/vpn/plugins/vpnc.c b/vpn/plugins/vpnc.c
index f222196a..7d3c9418 100644
--- a/vpn/plugins/vpnc.c
+++ b/vpn/plugins/vpnc.c
@@ -606,28 +606,17 @@ static void request_input_credentials_reply(DBusMessage 
*reply, void *user_data)
        char *secret = NULL, *username = NULL, *password = NULL;
        const char *key;
        DBusMessageIter iter, dict;
-       DBusError error;
-       int err_int = 0;
+       int err_int;
 
        DBG("provider %p", data->provider);
 
-       dbus_error_init(&error);
-
-       if (dbus_set_error_from_message(&error, reply)) {
-               if (!g_strcmp0(error.name, VPN_AGENT_INTERFACE
-                                                       ".Error.Canceled"))
-                       err_int = ECONNABORTED;
-
-               if (!g_strcmp0(error.name, VPN_AGENT_INTERFACE
-                                                       ".Error.Timeout"))
-                       err_int = ETIMEDOUT;
-
-               dbus_error_free(&error);
-
-               if (err_int)
-                       goto abort;
-
-               goto err;
+       err_int = vpn_agent_check_and_process_reply_error(reply, data->provider,
+                               data->task, data->cb, data->user_data);
+       if (err_int) {
+               /* Ensure cb is called only once */
+               data->cb = NULL;
+               data->user_data = NULL;
+               return;
        }
 
        if (!vpn_agent_check_reply_has_dict(reply))
@@ -697,20 +686,7 @@ static void request_input_credentials_reply(DBusMessage 
*reply, void *user_data)
 
 err:
        err_int = EACCES;
-
-abort:
        vc_connect_done(data, err_int);
-
-       switch (err_int) {
-       case EACCES:
-               vpn_provider_indicate_error(data->provider,
-                                       VPN_PROVIDER_ERROR_AUTH_FAILED);
-               break;
-       case ECONNABORTED:
-       case ETIMEDOUT:
-               vpn_provider_indicate_error(data->provider,
-                                       VPN_PROVIDER_ERROR_UNKNOWN);
-       }
 }
 
 static int request_input_credentials(struct vc_private_data *data,
-- 
2.20.1



------------------------------

Subject: Digest Footer

_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman


------------------------------

End of connman Digest, Vol 44, Issue 7
**************************************

Reply via email to