* [PATCH 01/45] agent: Cancel agent request on NoReply D-Bus error
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
@ 2025-07-11 14:26 ` Jussi Laakkonen
2025-07-11 14:26 ` [PATCH 02/45] vpn-provider: Use association state for VPN agent input wait Jussi Laakkonen
` (46 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:26 UTC (permalink / raw)
To: connman
Handle also the NoReply D-Bus error as this is commonly sent back when
the timeout set for the request is exceeded. Canceling the request later
becomes impossible as agent->pending will be set to NULL in
agent_finalize_pending(). Thus, making later calls to
connman_agent_cancel() to not to close down agent dialogs but instead
they are piled up on top of each other.
---
src/agent.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/agent.c b/src/agent.c
index 23517d9b..e2d1ef09 100644
--- a/src/agent.c
+++ b/src/agent.c
@@ -201,7 +201,9 @@ static void agent_receive_message(DBusPendingCall *call, void *user_data)
if (dbus_message_is_error(reply,
"org.freedesktop.DBus.Error.Timeout") ||
dbus_message_is_error(reply,
- "org.freedesktop.DBus.Error.TimedOut")) {
+ "org.freedesktop.DBus.Error.TimedOut") ||
+ dbus_message_is_error(reply,
+ "org.freedesktop.DBus.Error.NoReply")) {
send_cancel_request(agent, agent->pending);
}
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 02/45] vpn-provider: Use association state for VPN agent input wait
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
2025-07-11 14:26 ` [PATCH 01/45] agent: Cancel agent request on NoReply D-Bus error Jussi Laakkonen
@ 2025-07-11 14:26 ` Jussi Laakkonen
2025-07-11 14:26 ` [PATCH 03/45] vpn: Add association state before connect state Jussi Laakkonen
` (45 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:26 UTC (permalink / raw)
To: connman
Use the association state with VPNs to define that VPN is waiting for
input via agent. The same state is used for every service in connmand so
this change synchronizes the states in both.
Set the state to be identical to connmand side states by injecting this
into the VPN state machine before the connect state ("configuration"
state). This is then changed when the state is set to connected either
by getting a non-error reply from VPN agent or via VPN driver gets
connect state notify.
In this is association state the VPN indicates to connmand that the VPN
is requesting user input via agent and shouldn't be subject to connect
timeout checks. Having this additional state allows to obey the D-Bus VPN
agent query timeout value, instead of getting the dialog shut down at
connection timeout.
---
vpn/vpn-provider.c | 45 +++++++++++++++++++++++++++++++++++++++++----
vpn/vpn-provider.h | 6 ++++++
2 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index 4bcb8373..56040e65 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -1487,6 +1487,23 @@ int __vpn_provider_disconnect(struct vpn_provider *provider)
return err;
}
+static bool is_connected_state(enum vpn_provider_state state)
+{
+ switch (state) {
+ case VPN_PROVIDER_STATE_UNKNOWN:
+ case VPN_PROVIDER_STATE_IDLE:
+ case VPN_PROVIDER_STATE_DISCONNECT:
+ case VPN_PROVIDER_STATE_FAILURE:
+ break;
+ case VPN_PROVIDER_STATE_CONNECT:
+ case VPN_PROVIDER_STATE_READY:
+ case VPN_PROVIDER_STATE_ASSOCIATION:
+ return true;
+ }
+
+ return false;
+}
+
static void connect_cb(struct vpn_provider *provider, void *user_data,
int error)
{
@@ -1509,6 +1526,8 @@ static void connect_cb(struct vpn_provider *provider, void *user_data,
* No reply, disconnect called by connmand because of
* connection timeout.
*/
+ vpn_provider_indicate_error(provider,
+ VPN_PROVIDER_ERROR_CONNECT_FAILED);
break;
case ENOMSG:
/* fall through */
@@ -1533,9 +1552,7 @@ static void connect_cb(struct vpn_provider *provider, void *user_data,
* 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 (is_connected_state(provider->state)) {
if (provider->driver->set_state)
provider->driver->set_state(provider,
VPN_PROVIDER_STATE_DISCONNECT);
@@ -1597,6 +1614,17 @@ int __vpn_provider_connect(struct vpn_provider *provider, DBusMessage *msg)
if (reply)
g_dbus_send_message(connection, reply);
+ return -EINPROGRESS;
+ case VPN_PROVIDER_STATE_ASSOCIATION:
+ /*
+ * Do not interrupt user when inputting credentials via agent.
+ * The driver is in CONNECT state that would return EINPROGRESS
+ * and change provider state to CONNECT.
+ */
+ reply = __connman_error_in_progress(msg);
+ if (reply)
+ g_dbus_send_message(connection, reply);
+
return -EINPROGRESS;
case VPN_PROVIDER_STATE_UNKNOWN:
case VPN_PROVIDER_STATE_IDLE:
@@ -1626,7 +1654,7 @@ int __vpn_provider_connect(struct vpn_provider *provider, DBusMessage *msg)
return -EOPNOTSUPP;
if (err == -EINPROGRESS)
- vpn_provider_set_state(provider, VPN_PROVIDER_STATE_CONNECT);
+ vpn_provider_set_state(provider, VPN_PROVIDER_STATE_ASSOCIATION);
return err;
}
@@ -1767,6 +1795,8 @@ static const char *state2string(enum vpn_provider_state state)
break;
case VPN_PROVIDER_STATE_IDLE:
return "idle";
+ case VPN_PROVIDER_STATE_ASSOCIATION:
+ return "association";
case VPN_PROVIDER_STATE_CONNECT:
return "configuration";
case VPN_PROVIDER_STATE_READY:
@@ -1875,6 +1905,9 @@ static void append_state(DBusMessageIter *iter,
case VPN_PROVIDER_STATE_IDLE:
str = "idle";
break;
+ case VPN_PROVIDER_STATE_ASSOCIATION:
+ str = "association";
+ break;
case VPN_PROVIDER_STATE_CONNECT:
str = "configuration";
break;
@@ -2026,6 +2059,10 @@ int vpn_provider_set_state(struct vpn_provider *provider,
case VPN_PROVIDER_STATE_IDLE:
return set_connected(provider, false);
case VPN_PROVIDER_STATE_CONNECT:
+ if (provider->driver && provider->driver->set_state)
+ provider->driver->set_state(provider, state);
+ return provider_indicate_state(provider, state);
+ case VPN_PROVIDER_STATE_ASSOCIATION:
return provider_indicate_state(provider, state);
case VPN_PROVIDER_STATE_READY:
return set_connected(provider, true);
diff --git a/vpn/vpn-provider.h b/vpn/vpn-provider.h
index 5d1455da..c81476c6 100644
--- a/vpn/vpn-provider.h
+++ b/vpn/vpn-provider.h
@@ -44,6 +44,12 @@ enum vpn_provider_state {
VPN_PROVIDER_STATE_READY = 3,
VPN_PROVIDER_STATE_DISCONNECT = 4,
VPN_PROVIDER_STATE_FAILURE = 5,
+ /*
+ * Special state to indicate that user interaction is being waited for
+ * and disconnect timeout in connmand should not terminate this VPN but
+ * to let the agent timeout handle the case.
+ */
+ VPN_PROVIDER_STATE_ASSOCIATION = 6,
};
enum vpn_provider_error {
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 03/45] vpn: Add association state before connect state
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
2025-07-11 14:26 ` [PATCH 01/45] agent: Cancel agent request on NoReply D-Bus error Jussi Laakkonen
2025-07-11 14:26 ` [PATCH 02/45] vpn-provider: Use association state for VPN agent input wait Jussi Laakkonen
@ 2025-07-11 14:26 ` Jussi Laakkonen
2025-07-11 14:26 ` [PATCH 04/45] vpn-agent: Do connect state transition after input dialog check Jussi Laakkonen
` (44 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:26 UTC (permalink / raw)
To: connman
This changes the state machine by adding the VPN_STATE_ASSOCIATION to be
entered right after connect() callback is called. This is needed in
order to properly react with the user input dialog waiting on VPNs.
connect state is now set when the dialog is closed to indicate that user
input is given and now the VPN really connects.
When VPN notify() allback is called the connect state is enforced if the
return value indicates so and the internal state is different. This is
to accommodate the changes required and to operate as a fallback that
the states of provider and driver are kept in sync.
Warn about invalid transition to ASSOCIATION state in case vpn_notify()
gets it as a reply back from plugin notify.
---
vpn/plugins/vpn.c | 22 +++++++++++++++++++++-
vpn/plugins/vpn.h | 11 ++++++-----
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/vpn/plugins/vpn.c b/vpn/plugins/vpn.c
index 1644e16d..52f7cac7 100644
--- a/vpn/plugins/vpn.c
+++ b/vpn/plugins/vpn.c
@@ -220,6 +220,9 @@ static int vpn_set_state(struct vpn_provider *provider,
case VPN_PROVIDER_STATE_IDLE:
data->state = VPN_STATE_IDLE;
break;
+ case VPN_PROVIDER_STATE_ASSOCIATION:
+ data->state = VPN_STATE_ASSOCIATION;
+ break;
case VPN_PROVIDER_STATE_CONNECT:
case VPN_PROVIDER_STATE_READY:
data->state = VPN_STATE_CONNECT;
@@ -282,6 +285,12 @@ static DBusMessage *vpn_notify(struct connman_task *task,
switch (state) {
case VPN_STATE_CONNECT:
+ if (data->state == VPN_STATE_ASSOCIATION) {
+ data->state = VPN_STATE_CONNECT;
+ vpn_provider_set_state(provider,
+ VPN_PROVIDER_STATE_CONNECT);
+ }
+ /* fall through */
case VPN_STATE_READY:
if (data->state == VPN_STATE_READY) {
/*
@@ -334,6 +343,16 @@ static DBusMessage *vpn_notify(struct connman_task *task,
break;
case VPN_STATE_UNKNOWN:
+ break;
+
+ /* State transition to ASSOCIATION via notify is not allowed */
+ case VPN_STATE_ASSOCIATION:
+ connman_warn("Invalid %s vpn_notify() state transition "
+ "from %d to %d (ASSOCIATION)."
+ "VPN provider %p is disconnected",
+ vpn_driver_data->name, data->state,
+ state, provider);
+ /* fall through */
case VPN_STATE_IDLE:
case VPN_STATE_DISCONNECT:
case VPN_STATE_FAILURE:
@@ -568,6 +587,7 @@ static int vpn_connect(struct vpn_provider *provider,
data->state = VPN_STATE_IDLE;
break;
+ case VPN_STATE_ASSOCIATION:
case VPN_STATE_CONNECT:
return -EINPROGRESS;
@@ -648,7 +668,7 @@ static int vpn_connect(struct vpn_provider *provider,
DBG("%s started with dev %s",
vpn_driver_data->provider_driver.name, data->if_name);
- data->state = VPN_STATE_CONNECT;
+ data->state = VPN_STATE_ASSOCIATION;
return -EINPROGRESS;
diff --git a/vpn/plugins/vpn.h b/vpn/plugins/vpn.h
index fd10addf..a8d24fc3 100644
--- a/vpn/plugins/vpn.h
+++ b/vpn/plugins/vpn.h
@@ -34,11 +34,12 @@ extern "C" {
enum vpn_state {
VPN_STATE_UNKNOWN = 0,
VPN_STATE_IDLE = 1,
- VPN_STATE_CONNECT = 2,
- VPN_STATE_READY = 3,
- VPN_STATE_DISCONNECT = 4,
- VPN_STATE_FAILURE = 5,
- VPN_STATE_AUTH_FAILURE = 6,
+ VPN_STATE_ASSOCIATION = 2,
+ VPN_STATE_CONNECT = 3,
+ VPN_STATE_READY = 4,
+ VPN_STATE_DISCONNECT = 5,
+ VPN_STATE_FAILURE = 6,
+ VPN_STATE_AUTH_FAILURE = 7,
};
struct vpn_driver {
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 04/45] vpn-agent: Do connect state transition after input dialog check
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (2 preceding siblings ...)
2025-07-11 14:26 ` [PATCH 03/45] vpn: Add association state before connect state Jussi Laakkonen
@ 2025-07-11 14:26 ` Jussi Laakkonen
2025-07-11 14:26 ` [PATCH 05/45] service: Explicit VPN connect timeout, ignore in VPN agent wait Jussi Laakkonen
` (43 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:26 UTC (permalink / raw)
To: connman
When the VPN requests input (credentials) via the VPN agent the
vpn_agent_check_and_process_reply_error() does transition the state of
the VPN provider to connect state when there is no error. This is done
to facilitate the transition from the association state to connect
state as each VPN should use this function to verify the D-Bus reply
and, thus will be called after each reply.
---
vpn/vpn-agent.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/vpn/vpn-agent.c b/vpn/vpn-agent.c
index ab6fea55..f1cc7e36 100644
--- a/vpn/vpn-agent.c
+++ b/vpn/vpn-agent.c
@@ -257,8 +257,12 @@ int vpn_agent_check_and_process_reply_error(DBusMessage *reply,
dbus_error_init(&error);
- if (!dbus_set_error_from_message(&error, reply))
+ if (!dbus_set_error_from_message(&error, reply)) {
+ DBG("Dialog without error, set provider %p to CONNECT",
+ provider);
+ vpn_provider_set_state(provider, VPN_PROVIDER_STATE_CONNECT);
return 0;
+ }
if (!g_strcmp0(error.name, VPN_AGENT_INTERFACE ".Error.Canceled"))
err = ECANCELED;
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 05/45] service: Explicit VPN connect timeout, ignore in VPN agent wait
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (3 preceding siblings ...)
2025-07-11 14:26 ` [PATCH 04/45] vpn-agent: Do connect state transition after input dialog check Jussi Laakkonen
@ 2025-07-11 14:26 ` Jussi Laakkonen
2025-08-01 19:31 ` Denis Kenzior
2025-07-11 14:26 ` [PATCH 06/45] provider: Handle VPN configuration and association states Jussi Laakkonen
` (42 subsequent siblings)
47 siblings, 1 reply; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:26 UTC (permalink / raw)
To: connman
Ignore the connect timeout autostarting when connecting a VPN service
because initially the VPN is in association state in which the VPN is
waiting for the VPN agent. Separate the starting of connect timeout into
its own function __connman_service_start_connect_timeout() so provider.c
can call it when it enters configuration state.
When a VPN is waiting for user input it should not be affected by
connect timeout as the connection is not yet attempted. This may happen
if VPN resumes to association state when requiring the VPN agent for
other, e.g., encrypted private key input after credential input.
---
src/connman.h | 2 ++
src/service.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/src/connman.h b/src/connman.h
index 7ebda7af..1a8952e4 100644
--- a/src/connman.h
+++ b/src/connman.h
@@ -788,6 +788,8 @@ int __connman_service_connect(struct connman_service *service,
int __connman_service_disconnect(struct connman_service *service);
void __connman_service_set_active_session(bool enable, GSList *list);
void __connman_service_auto_connect(enum connman_service_connect_reason reason);
+void __connman_service_start_connect_timeout(struct connman_service *service,
+ bool restart);
bool __connman_service_remove(struct connman_service *service);
void __connman_service_set_hidden_data(struct connman_service *service,
gpointer user_data);
diff --git a/src/service.c b/src/service.c
index f73f42eb..9fd8fdb1 100644
--- a/src/service.c
+++ b/src/service.c
@@ -7322,8 +7322,27 @@ static gboolean connect_timeout(gpointer user_data)
if (service->network)
__connman_network_disconnect(service->network);
- else if (service->provider)
+ else if (service->provider) {
+ /*
+ * Remove timeout when the VPN is waiting for user input in
+ * association state. By default the VPN agent timeout is
+ * 300s whereas default connection timeout is 120s. Provider
+ * will start connect timeout for the service when it enters
+ * configuration state.
+ */
+ const char *statestr = connman_provider_get_string(
+ service->provider, "State");
+ if (!g_strcmp0(statestr, "association")) {
+ DBG("VPN provider %p is waiting for VPN agent, "
+ "stop connect timeout",
+ service->provider);
+ return G_SOURCE_REMOVE;
+ }
+
connman_provider_disconnect(service->provider);
+ }
+
+
__connman_stats_service_unregister(service);
@@ -7351,7 +7370,27 @@ static gboolean connect_timeout(gpointer user_data)
CONNMAN_SERVICE_CONNECT_REASON_USER)
do_auto_connect(service, CONNMAN_SERVICE_CONNECT_REASON_AUTO);
- return FALSE;
+ return G_SOURCE_REMOVE;
+}
+
+void __connman_service_start_connect_timeout(struct connman_service *service,
+ bool restart)
+{
+ DBG("");
+
+ if (!service)
+ return;
+
+ if (!restart && service->timeout)
+ return;
+
+ if (restart && service->timeout) {
+ DBG("cancel running connect timeout");
+ g_source_remove(service->timeout);
+ }
+
+ service->timeout = g_timeout_add_seconds(CONNECT_TIMEOUT,
+ connect_timeout, service);
}
static DBusMessage *connect_service(DBusConnection *conn,
@@ -9975,9 +10014,12 @@ int __connman_service_connect(struct connman_service *service,
return 0;
if (err == -EINPROGRESS) {
- if (service->timeout == 0)
- service->timeout = g_timeout_add_seconds(
- CONNECT_TIMEOUT, connect_timeout, service);
+ /*
+ * VPN will start connect timeout when it enters CONFIGURATION
+ * state.
+ */
+ if (service->type != CONNMAN_SERVICE_TYPE_VPN)
+ __connman_service_start_connect_timeout(service, false);
return -EINPROGRESS;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 05/45] service: Explicit VPN connect timeout, ignore in VPN agent wait
2025-07-11 14:26 ` [PATCH 05/45] service: Explicit VPN connect timeout, ignore in VPN agent wait Jussi Laakkonen
@ 2025-08-01 19:31 ` Denis Kenzior
2025-08-08 12:05 ` Jussi Laakkonen
0 siblings, 1 reply; 69+ messages in thread
From: Denis Kenzior @ 2025-08-01 19:31 UTC (permalink / raw)
To: Jussi Laakkonen, connman
Hi Jussi,
On 7/11/25 9:26 AM, Jussi Laakkonen wrote:
> Ignore the connect timeout autostarting when connecting a VPN service
> because initially the VPN is in association state in which the VPN is
> waiting for the VPN agent. Separate the starting of connect timeout into
> its own function __connman_service_start_connect_timeout() so provider.c
> can call it when it enters configuration state.
>
> When a VPN is waiting for user input it should not be affected by
> connect timeout as the connection is not yet attempted. This may happen
> if VPN resumes to association state when requiring the VPN agent for
> other, e.g., encrypted private key input after credential input.
> ---
> src/connman.h | 2 ++
> src/service.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++-----
> 2 files changed, 49 insertions(+), 5 deletions(-)
>
> diff --git a/src/connman.h b/src/connman.h
> index 7ebda7af..1a8952e4 100644
> --- a/src/connman.h
> +++ b/src/connman.h
> @@ -788,6 +788,8 @@ int __connman_service_connect(struct connman_service *service,
> int __connman_service_disconnect(struct connman_service *service);
> void __connman_service_set_active_session(bool enable, GSList *list);
> void __connman_service_auto_connect(enum connman_service_connect_reason reason);
> +void __connman_service_start_connect_timeout(struct connman_service *service,
> + bool restart);
> bool __connman_service_remove(struct connman_service *service);
> void __connman_service_set_hidden_data(struct connman_service *service,
> gpointer user_data);
> diff --git a/src/service.c b/src/service.c
> index f73f42eb..9fd8fdb1 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -7322,8 +7322,27 @@ static gboolean connect_timeout(gpointer user_data)
>
> if (service->network)
> __connman_network_disconnect(service->network);
> - else if (service->provider)
> + else if (service->provider) {
> + /*
> + * Remove timeout when the VPN is waiting for user input in
> + * association state. By default the VPN agent timeout is
> + * 300s whereas default connection timeout is 120s. Provider
> + * will start connect timeout for the service when it enters
> + * configuration state.
> + */
> + const char *statestr = connman_provider_get_string(
> + service->provider, "State");
> + if (!g_strcmp0(statestr, "association")) {
> + DBG("VPN provider %p is waiting for VPN agent, "
> + "stop connect timeout",
> + service->provider);
> + return G_SOURCE_REMOVE;
> + }
> +
I'm a bit confused why this entire block is needed? If the service is a VPN,
then the timeout isn't even started, and thus connect_timeout() shouldn't even
be called?
> connman_provider_disconnect(service->provider);
> + }
> +
> +
>
> __connman_stats_service_unregister(service);
>
> @@ -7351,7 +7370,27 @@ static gboolean connect_timeout(gpointer user_data)
> CONNMAN_SERVICE_CONNECT_REASON_USER)
> do_auto_connect(service, CONNMAN_SERVICE_CONNECT_REASON_AUTO);
>
> - return FALSE;
> + return G_SOURCE_REMOVE;
> +}
> +
> +void __connman_service_start_connect_timeout(struct connman_service *service,
> + bool restart)
> +{
> + DBG("");
> +
> + if (!service)
> + return;
> +
> + if (!restart && service->timeout)
> + return;
> +
> + if (restart && service->timeout) {
> + DBG("cancel running connect timeout");
> + g_source_remove(service->timeout);
> + }
> +
> + service->timeout = g_timeout_add_seconds(CONNECT_TIMEOUT,
> + connect_timeout, service);
> }
Given the above, I question the need for this function?
>
> static DBusMessage *connect_service(DBusConnection *conn,
> @@ -9975,9 +10014,12 @@ int __connman_service_connect(struct connman_service *service,
> return 0;
>
> if (err == -EINPROGRESS) {
> - if (service->timeout == 0)
> - service->timeout = g_timeout_add_seconds(
> - CONNECT_TIMEOUT, connect_timeout, service);
> + /*
> + * VPN will start connect timeout when it enters CONFIGURATION
> + * state.
> + */
> + if (service->type != CONNMAN_SERVICE_TYPE_VPN)
> + __connman_service_start_connect_timeout(service, false);
>
> return -EINPROGRESS;
> }
Regards,
-Denis
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 05/45] service: Explicit VPN connect timeout, ignore in VPN agent wait
2025-08-01 19:31 ` Denis Kenzior
@ 2025-08-08 12:05 ` Jussi Laakkonen
2025-08-08 16:08 ` Denis Kenzior
0 siblings, 1 reply; 69+ messages in thread
From: Jussi Laakkonen @ 2025-08-08 12:05 UTC (permalink / raw)
To: Denis Kenzior, connman
Hi Denis,
I'll try to remember this from 3 years back when this was done.
On 8/1/25 22:31, Denis Kenzior wrote:
> Hi Jussi,
>
> On 7/11/25 9:26 AM, Jussi Laakkonen wrote:
>> Ignore the connect timeout autostarting when connecting a VPN service
>> because initially the VPN is in association state in which the VPN is
>> waiting for the VPN agent. Separate the starting of connect timeout into
>> its own function __connman_service_start_connect_timeout() so provider.c
>> can call it when it enters configuration state.
>>
>> When a VPN is waiting for user input it should not be affected by
>> connect timeout as the connection is not yet attempted. This may happen
>> if VPN resumes to association state when requiring the VPN agent for
>> other, e.g., encrypted private key input after credential input.
>> ---
>> src/connman.h | 2 ++
>> src/service.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++-----
>> 2 files changed, 49 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/connman.h b/src/connman.h
>> index 7ebda7af..1a8952e4 100644
>> --- a/src/connman.h
>> +++ b/src/connman.h
>> @@ -788,6 +788,8 @@ int __connman_service_connect(struct
>> connman_service *service,
>> int __connman_service_disconnect(struct connman_service *service);
>> void __connman_service_set_active_session(bool enable, GSList *list);
>> void __connman_service_auto_connect(enum
>> connman_service_connect_reason reason);
>> +void __connman_service_start_connect_timeout(struct connman_service
>> *service,
>> + bool restart);
>> bool __connman_service_remove(struct connman_service *service);
>> void __connman_service_set_hidden_data(struct connman_service *service,
>> gpointer user_data);
>> diff --git a/src/service.c b/src/service.c
>> index f73f42eb..9fd8fdb1 100644
>> --- a/src/service.c
>> +++ b/src/service.c
>> @@ -7322,8 +7322,27 @@ static gboolean connect_timeout(gpointer
>> user_data)
>> if (service->network)
>> __connman_network_disconnect(service->network);
>> - else if (service->provider)
>> + else if (service->provider) {
>> + /*
>> + * Remove timeout when the VPN is waiting for user input in
>> + * association state. By default the VPN agent timeout is
>> + * 300s whereas default connection timeout is 120s. Provider
>> + * will start connect timeout for the service when it enters
>> + * configuration state.
>> + */
>> + const char *statestr = connman_provider_get_string(
>> + service->provider, "State");
>> + if (!g_strcmp0(statestr, "association")) {
>> + DBG("VPN provider %p is waiting for VPN agent, "
>> + "stop connect timeout",
>> + service->provider);
>> + return G_SOURCE_REMOVE;
>> + }
>> +
>
> I'm a bit confused why this entire block is needed? If the service is a
> VPN, then the timeout isn't even started, and thus connect_timeout()
> shouldn't even be called?
Yes and the point of this was to extend that the provider.c can initiate
the timeout for the VPNs as well to behave similarly to other services.
So In ASSOCIATION state there is the 300 timeout for inputting
credentials etc. which is then followed by the 120 timeout for
connecting in CONFIGURATION state.
I think this was done because the agent needed a bit longer timeout than
the connection timeout is for the user to input the credentials and it
seemed to obey the 120 timeout instead, so the aim was to make VPNs to
have same states as any other service, to synchronize the behavior.
>
>> connman_provider_disconnect(service->provider);
>> + }
>> +
>> +
>> __connman_stats_service_unregister(service);
>> @@ -7351,7 +7370,27 @@ static gboolean connect_timeout(gpointer
>> user_data)
>> CONNMAN_SERVICE_CONNECT_REASON_USER)
>> do_auto_connect(service, CONNMAN_SERVICE_CONNECT_REASON_AUTO);
>> - return FALSE;
>> + return G_SOURCE_REMOVE;
>> +}
>> +
>> +void __connman_service_start_connect_timeout(struct connman_service
>> *service,
>> + bool restart)
>> +{
>> + DBG("");
>> +
>> + if (!service)
>> + return;
>> +
>> + if (!restart && service->timeout)
>> + return;
>> +
>> + if (restart && service->timeout) {
>> + DBG("cancel running connect timeout");
>> + g_source_remove(service->timeout);
>> + }
>> +
>> + service->timeout = g_timeout_add_seconds(CONNECT_TIMEOUT,
>> + connect_timeout, service);
>> }
>
> Given the above, I question the need for this function?
This is needed for keeping the behavior of services in sync, not to
separate VPNs.
>
>> static DBusMessage *connect_service(DBusConnection *conn,
>> @@ -9975,9 +10014,12 @@ int __connman_service_connect(struct
>> connman_service *service,
>> return 0;
>> if (err == -EINPROGRESS) {
>> - if (service->timeout == 0)
>> - service->timeout = g_timeout_add_seconds(
>> - CONNECT_TIMEOUT, connect_timeout, service);
>> + /*
>> + * VPN will start connect timeout when it enters CONFIGURATION
>> + * state.
>> + */
>> + if (service->type != CONNMAN_SERVICE_TYPE_VPN)
>> + __connman_service_start_connect_timeout(service, false);
>> return -EINPROGRESS;
>> }
>
> Regards,
> -Denis
BR,
Jussi
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 05/45] service: Explicit VPN connect timeout, ignore in VPN agent wait
2025-08-08 12:05 ` Jussi Laakkonen
@ 2025-08-08 16:08 ` Denis Kenzior
2025-08-11 14:21 ` Jussi Laakkonen
0 siblings, 1 reply; 69+ messages in thread
From: Denis Kenzior @ 2025-08-08 16:08 UTC (permalink / raw)
To: Jussi Laakkonen, connman
Hi Jussi,
>>> @@ -7322,8 +7322,27 @@ static gboolean connect_timeout(gpointer user_data)
>>> if (service->network)
>>> __connman_network_disconnect(service->network);
>>> - else if (service->provider)
>>> + else if (service->provider) {
>>> + /*
>>> + * Remove timeout when the VPN is waiting for user input in
>>> + * association state. By default the VPN agent timeout is
>>> + * 300s whereas default connection timeout is 120s. Provider
>>> + * will start connect timeout for the service when it enters
>>> + * configuration state.
>>> + */
>>> + const char *statestr = connman_provider_get_string(
>>> + service->provider, "State");
>>> + if (!g_strcmp0(statestr, "association")) {
>>> + DBG("VPN provider %p is waiting for VPN agent, "
>>> + "stop connect timeout",
>>> + service->provider);
>>> + return G_SOURCE_REMOVE;
>>> + }
>>> +
>>
>> I'm a bit confused why this entire block is needed? If the service is a VPN,
>> then the timeout isn't even started, and thus connect_timeout() shouldn't even
>> be called?
>
> Yes and the point of this was to extend that the provider.c can initiate the
> timeout for the VPNs as well to behave similarly to other services. So In
> ASSOCIATION state there is the 300 timeout for inputting credentials etc. which
> is then followed by the 120 timeout for connecting in CONFIGURATION state.
Yeah, but right now you're introducing something that is essentially dead code?
How is this a good idea? Introduce it in a patch where this is actually needed?
> > I think this was done because the agent needed a bit longer timeout than the
> connection timeout is for the user to input the credentials and it seemed to
> obey the 120 timeout instead, so the aim was to make VPNs to have same states as
> any other service, to synchronize the behavior.
>
Why would the core have a timeout for this at all? NoReply timeouts are purely
a libdbus-1 construct. You could have a dialog sitting there forever asking
user to input username/password prior to initiating a connection. The driver
should really figure out whether a timeout is needed or not.
Regards,
-Denis
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 05/45] service: Explicit VPN connect timeout, ignore in VPN agent wait
2025-08-08 16:08 ` Denis Kenzior
@ 2025-08-11 14:21 ` Jussi Laakkonen
0 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-08-11 14:21 UTC (permalink / raw)
To: Denis Kenzior, connman
Hi Denis,
On 8/8/25 19:08, Denis Kenzior wrote:
> Hi Jussi,
>
> >>> @@ -7322,8 +7322,27 @@ static gboolean connect_timeout(gpointer
> user_data)
>>>> if (service->network)
>>>> __connman_network_disconnect(service->network);
>>>> - else if (service->provider)
>>>> + else if (service->provider) {
>>>> + /*
>>>> + * Remove timeout when the VPN is waiting for user input in
>>>> + * association state. By default the VPN agent timeout is
>>>> + * 300s whereas default connection timeout is 120s. Provider
>>>> + * will start connect timeout for the service when it enters
>>>> + * configuration state.
>>>> + */
>>>> + const char *statestr = connman_provider_get_string(
>>>> + service->provider, "State");
>>>> + if (!g_strcmp0(statestr, "association")) {
>>>> + DBG("VPN provider %p is waiting for VPN agent, "
>>>> + "stop connect timeout",
>>>> + service->provider);
>>>> + return G_SOURCE_REMOVE;
>>>> + }
>>>> +
>>>
>>> I'm a bit confused why this entire block is needed? If the service
>>> is a VPN, then the timeout isn't even started, and thus
>>> connect_timeout() shouldn't even be called?
>>
>> Yes and the point of this was to extend that the provider.c can
>> initiate the timeout for the VPNs as well to behave similarly to other
>> services. So In ASSOCIATION state there is the 300 timeout for
>> inputting credentials etc. which is then followed by the 120 timeout
>> for connecting in CONFIGURATION state.
>
> Yeah, but right now you're introducing something that is essentially
> dead code? How is this a good idea? Introduce it in a patch where this
> is actually needed?
>
> > > I think this was done because the agent needed a bit longer timeout
> than the
>> connection timeout is for the user to input the credentials and it
>> seemed to obey the 120 timeout instead, so the aim was to make VPNs to
>> have same states as any other service, to synchronize the behavior.
>>
>
> Why would the core have a timeout for this at all? NoReply timeouts are
> purely a libdbus-1 construct. You could have a dialog sitting there
> forever asking user to input username/password prior to initiating a
> connection. The driver should really figure out whether a timeout is
> needed or not.
>
Ok, now I remember why this was done. Both connmand and vpnd have
separate and configurable timeouts [1,2]. And by looking at the code,
any service started will have the connection timeout set [3].
The main reason, with adding the association state to VPN, both can be
respected and VPNs will have same characteristics regarding timeouts as
any other service, except that the vpnd side timeout is now respected in
the code and these two do not overlap. It was a bit puzzling to find
this out in the first place that why the changes to vpnd timeout value
has no effect, as, for example, NordVPN credentials are so long and
since our UI implementation would not bend anymore the timeout for input
of connmand was not enough for inputting them. And since both had
separate configuration it did not seem feasible to increase the connmand
side because of this but make them both work, independently of each
other. Thus, I think this addresses the issue of these two separate
timeouts.
Granted, the __connman_service_start_connect_timeout() could have the
checks that if (vpn && state < CONFIGURATION) ; return; or something.
This part of the patch set was already discussed with earlier maintainer
[4] but because of reasons I had no time to do the regular, non-RFC
patch set yet.
BR,
Jussi
[1]
https://git.kernel.org/pub/scm/network/connman/connman.git/tree/doc/connman.conf.5.in#n29
[2]
https://git.kernel.org/pub/scm/network/connman/connman.git/tree/doc/connman-vpn.conf.5.in#n41
[3]
https://git.kernel.org/pub/scm/network/connman/connman.git/tree/src/service.c#n9924
[4]
https://lore.kernel.org/connman/20221213140707.278313-1-jussi.laakkonen@jolla.com/
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 06/45] provider: Handle VPN configuration and association states
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (4 preceding siblings ...)
2025-07-11 14:26 ` [PATCH 05/45] service: Explicit VPN connect timeout, ignore in VPN agent wait Jussi Laakkonen
@ 2025-07-11 14:26 ` Jussi Laakkonen
2025-08-01 19:34 ` Denis Kenzior
2025-07-11 14:26 ` [PATCH 07/45] vpn: Add support for association state, add state getter Jussi Laakkonen
` (41 subsequent siblings)
47 siblings, 1 reply; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:26 UTC (permalink / raw)
To: connman
Set the association state when VPN is waiting for user input as an
initial state after connecting the provider. Set the configuration
state (as it is declaced to be the string to connect state in VPN)
accordingly as well. Start VPN connect timeout in configuration
state with restart option to ensure that the timeout begins from the
last known configuration (connect) state.
---
include/provider.h | 9 +++++----
src/provider.c | 22 +++++++++++++++++++++-
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/include/provider.h b/include/provider.h
index 3f2e36ad..aac47527 100644
--- a/include/provider.h
+++ b/include/provider.h
@@ -44,10 +44,11 @@ enum connman_provider_type {
enum connman_provider_state {
CONNMAN_PROVIDER_STATE_UNKNOWN = 0,
CONNMAN_PROVIDER_STATE_IDLE = 1,
- CONNMAN_PROVIDER_STATE_CONNECT = 2,
- CONNMAN_PROVIDER_STATE_READY = 3,
- CONNMAN_PROVIDER_STATE_DISCONNECT = 4,
- CONNMAN_PROVIDER_STATE_FAILURE = 5,
+ CONNMAN_PROVIDER_STATE_ASSOCIATION = 2,
+ CONNMAN_PROVIDER_STATE_CONNECT = 3,
+ CONNMAN_PROVIDER_STATE_READY = 4,
+ CONNMAN_PROVIDER_STATE_DISCONNECT = 5,
+ CONNMAN_PROVIDER_STATE_FAILURE = 6,
};
enum connman_provider_error {
diff --git a/src/provider.c b/src/provider.c
index 1f0ce10d..ab4aeafb 100644
--- a/src/provider.c
+++ b/src/provider.c
@@ -126,6 +126,22 @@ static int provider_indicate_state(struct connman_provider *provider,
{
DBG("state %d", state);
+ switch (state) {
+ case CONNMAN_SERVICE_STATE_UNKNOWN:
+ case CONNMAN_SERVICE_STATE_IDLE:
+ case CONNMAN_SERVICE_STATE_ASSOCIATION:
+ break;
+ case CONNMAN_SERVICE_STATE_CONFIGURATION:
+ __connman_service_start_connect_timeout(provider->vpn_service,
+ true);
+ break;
+ case CONNMAN_SERVICE_STATE_READY:
+ case CONNMAN_SERVICE_STATE_ONLINE:
+ case CONNMAN_SERVICE_STATE_DISCONNECT:
+ case CONNMAN_SERVICE_STATE_FAILURE:
+ break;
+ }
+
__connman_service_ipconfig_indicate_state(provider->vpn_service, state,
CONNMAN_IPCONFIG_TYPE_IPV4);
@@ -291,9 +307,13 @@ int connman_provider_set_state(struct connman_provider *provider,
return -EINVAL;
case CONNMAN_PROVIDER_STATE_IDLE:
return set_connected(provider, false);
- case CONNMAN_PROVIDER_STATE_CONNECT:
+ case CONNMAN_PROVIDER_STATE_ASSOCIATION:
+ /* Connect timeout is not effective for VPNs in this state */
return provider_indicate_state(provider,
CONNMAN_SERVICE_STATE_ASSOCIATION);
+ case CONNMAN_PROVIDER_STATE_CONNECT:
+ return provider_indicate_state(provider,
+ CONNMAN_SERVICE_STATE_CONFIGURATION);
case CONNMAN_PROVIDER_STATE_READY:
return set_connected(provider, true);
case CONNMAN_PROVIDER_STATE_DISCONNECT:
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 06/45] provider: Handle VPN configuration and association states
2025-07-11 14:26 ` [PATCH 06/45] provider: Handle VPN configuration and association states Jussi Laakkonen
@ 2025-08-01 19:34 ` Denis Kenzior
2025-08-08 12:12 ` Jussi Laakkonen
0 siblings, 1 reply; 69+ messages in thread
From: Denis Kenzior @ 2025-08-01 19:34 UTC (permalink / raw)
To: Jussi Laakkonen, connman
Hi Jussi,
On 7/11/25 9:26 AM, Jussi Laakkonen wrote:
> Set the association state when VPN is waiting for user input as an
> initial state after connecting the provider. Set the configuration
> state (as it is declaced to be the string to connect state in VPN)
> accordingly as well. Start VPN connect timeout in configuration
> state with restart option to ensure that the timeout begins from the
> last known configuration (connect) state.
> ---
> include/provider.h | 9 +++++----
> src/provider.c | 22 +++++++++++++++++++++-
> 2 files changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/include/provider.h b/include/provider.h
> index 3f2e36ad..aac47527 100644
> --- a/include/provider.h
> +++ b/include/provider.h
> @@ -44,10 +44,11 @@ enum connman_provider_type {
> enum connman_provider_state {
> CONNMAN_PROVIDER_STATE_UNKNOWN = 0,
> CONNMAN_PROVIDER_STATE_IDLE = 1,
> - CONNMAN_PROVIDER_STATE_CONNECT = 2,
> - CONNMAN_PROVIDER_STATE_READY = 3,
> - CONNMAN_PROVIDER_STATE_DISCONNECT = 4,
> - CONNMAN_PROVIDER_STATE_FAILURE = 5,
> + CONNMAN_PROVIDER_STATE_ASSOCIATION = 2,
> + CONNMAN_PROVIDER_STATE_CONNECT = 3,
> + CONNMAN_PROVIDER_STATE_READY = 4,
> + CONNMAN_PROVIDER_STATE_DISCONNECT = 5,
> + CONNMAN_PROVIDER_STATE_FAILURE = 6,
> };
>
> enum connman_provider_error {
> diff --git a/src/provider.c b/src/provider.c
> index 1f0ce10d..ab4aeafb 100644
> --- a/src/provider.c
> +++ b/src/provider.c
> @@ -126,6 +126,22 @@ static int provider_indicate_state(struct connman_provider *provider,
> {
> DBG("state %d", state);
>
> + switch (state) {
> + case CONNMAN_SERVICE_STATE_UNKNOWN:
> + case CONNMAN_SERVICE_STATE_IDLE:
> + case CONNMAN_SERVICE_STATE_ASSOCIATION:
> + break;
> + case CONNMAN_SERVICE_STATE_CONFIGURATION:
> + __connman_service_start_connect_timeout(provider->vpn_service,
> + true);
Possibly handle this inside service.c?
> + break;
> + case CONNMAN_SERVICE_STATE_READY:
> + case CONNMAN_SERVICE_STATE_ONLINE:
> + case CONNMAN_SERVICE_STATE_DISCONNECT:
> + case CONNMAN_SERVICE_STATE_FAILURE:
> + break;
> + }
> +
> __connman_service_ipconfig_indicate_state(provider->vpn_service, state,
> CONNMAN_IPCONFIG_TYPE_IPV4);
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 06/45] provider: Handle VPN configuration and association states
2025-08-01 19:34 ` Denis Kenzior
@ 2025-08-08 12:12 ` Jussi Laakkonen
0 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-08-08 12:12 UTC (permalink / raw)
To: Denis Kenzior, connman
Hi Denis,
On 8/1/25 22:34, Denis Kenzior wrote:
> Hi Jussi,
>
> On 7/11/25 9:26 AM, Jussi Laakkonen wrote:
>> Set the association state when VPN is waiting for user input as an
>> initial state after connecting the provider. Set the configuration
>> state (as it is declaced to be the string to connect state in VPN)
>> accordingly as well. Start VPN connect timeout in configuration
>> state with restart option to ensure that the timeout begins from the
>> last known configuration (connect) state.
>> ---
>> include/provider.h | 9 +++++----
>> src/provider.c | 22 +++++++++++++++++++++-
>> 2 files changed, 26 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/provider.h b/include/provider.h
>> index 3f2e36ad..aac47527 100644
>> --- a/include/provider.h
>> +++ b/include/provider.h
>> @@ -44,10 +44,11 @@ enum connman_provider_type {
>> enum connman_provider_state {
>> CONNMAN_PROVIDER_STATE_UNKNOWN = 0,
>> CONNMAN_PROVIDER_STATE_IDLE = 1,
>> - CONNMAN_PROVIDER_STATE_CONNECT = 2,
>> - CONNMAN_PROVIDER_STATE_READY = 3,
>> - CONNMAN_PROVIDER_STATE_DISCONNECT = 4,
>> - CONNMAN_PROVIDER_STATE_FAILURE = 5,
>> + CONNMAN_PROVIDER_STATE_ASSOCIATION = 2,
>> + CONNMAN_PROVIDER_STATE_CONNECT = 3,
>> + CONNMAN_PROVIDER_STATE_READY = 4,
>> + CONNMAN_PROVIDER_STATE_DISCONNECT = 5,
>> + CONNMAN_PROVIDER_STATE_FAILURE = 6,
>> };
>> enum connman_provider_error {
>> diff --git a/src/provider.c b/src/provider.c
>> index 1f0ce10d..ab4aeafb 100644
>> --- a/src/provider.c
>> +++ b/src/provider.c
>> @@ -126,6 +126,22 @@ static int provider_indicate_state(struct
>> connman_provider *provider,
>> {
>> DBG("state %d", state);
>> + switch (state) {
>> + case CONNMAN_SERVICE_STATE_UNKNOWN:
>> + case CONNMAN_SERVICE_STATE_IDLE:
>> + case CONNMAN_SERVICE_STATE_ASSOCIATION:
>> + break;
>> + case CONNMAN_SERVICE_STATE_CONFIGURATION:
>> + __connman_service_start_connect_timeout(provider->vpn_service,
>> + true);
>
> Possibly handle this inside service.c?
>
Could be. However, I kept this here mainly because the VPN related
operations can be extended based on the actual service state. Like, we
have an IPv6 disable option that was not yet accepted by upstream, which
simply disables IPv6 from the transport and all connected when VPN setup
requests so. It was discussed whether undef routes could be used but
after trying this approach, also with desktop, it seems that
applications work really weirdly with it. It is not related to this but
makes it more convenient to make certain VPN related actions within
provider.c and not to clutter service.c with more VPN specifics, right?
>> + break;
>> + case CONNMAN_SERVICE_STATE_READY:
>> + case CONNMAN_SERVICE_STATE_ONLINE:
>> + case CONNMAN_SERVICE_STATE_DISCONNECT:
>> + case CONNMAN_SERVICE_STATE_FAILURE:
>> + break;
>> + }
>> +
>> __connman_service_ipconfig_indicate_state(provider->vpn_service,
>> state,
>> CONNMAN_IPCONFIG_TYPE_IPV4);
>
> Regards,
> -Denis
BR,
Jussi
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 07/45] vpn: Add support for association state, add state getter
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (5 preceding siblings ...)
2025-07-11 14:26 ` [PATCH 06/45] provider: Handle VPN configuration and association states Jussi Laakkonen
@ 2025-07-11 14:26 ` Jussi Laakkonen
2025-07-11 14:26 ` [PATCH 08/45] vpn: Check if connecting when setting state or disconnecting Jussi Laakkonen
` (40 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:26 UTC (permalink / raw)
To: connman
Support VPN wait user input state as the association state.
Add support for "State" string into the get_property() driver callback.
---
plugins/vpn.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/plugins/vpn.c b/plugins/vpn.c
index 332d8e83..854b817f 100644
--- a/plugins/vpn.c
+++ b/plugins/vpn.c
@@ -156,6 +156,8 @@ static const char *get_string(struct connman_provider *provider,
return data->domain;
else if (g_str_equal(key, "Transport"))
return data->service_ident;
+ else if (g_str_equal(key, "State"))
+ return data->state;
return g_hash_table_lookup(data->setting_strings, key);
}
@@ -283,6 +285,8 @@ static void set_provider_state(struct connection_data *data)
goto set;
} else if (g_str_equal(data->state, "configuration")) {
state = CONNMAN_PROVIDER_STATE_CONNECT;
+ } else if (g_str_equal(data->state, "association")) {
+ state = CONNMAN_PROVIDER_STATE_ASSOCIATION;
} else if (g_str_equal(data->state, "idle")) {
state = CONNMAN_PROVIDER_STATE_IDLE;
} else if (g_str_equal(data->state, "disconnect")) {
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 08/45] vpn: Check if connecting when setting state or disconnecting
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (6 preceding siblings ...)
2025-07-11 14:26 ` [PATCH 07/45] vpn: Add support for association state, add state getter Jussi Laakkonen
@ 2025-07-11 14:26 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 09/45] vpn: Add VPN agent use callback for plugins Jussi Laakkonen
` (39 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:26 UTC (permalink / raw)
To: connman
Add checking of connected and connecting state in cases when the state
is being set and state transitions to disconnecting. This change avoids
clearing the transport ident when VPN is waiting for input from VPN
agent (association state).
---
plugins/vpn.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/plugins/vpn.c b/plugins/vpn.c
index 854b817f..d683c67d 100644
--- a/plugins/vpn.c
+++ b/plugins/vpn.c
@@ -270,6 +270,13 @@ static bool provider_is_connected(struct connection_data *data)
g_str_equal(data->state, "configuration"));
}
+static bool provider_is_connected_or_connecting(struct connection_data *data)
+{
+ return data && (g_str_equal(data->state, "ready") ||
+ g_str_equal(data->state, "configuration") ||
+ g_str_equal(data->state, "association"));
+}
+
static void set_provider_state(struct connection_data *data)
{
enum connman_provider_state state = CONNMAN_PROVIDER_STATE_UNKNOWN;
@@ -278,7 +285,11 @@ static void set_provider_state(struct connection_data *data)
DBG("provider %p new state %s", data->provider, data->state);
- connected = provider_is_connected(data);
+ /*
+ * To avoid clearing transport ident when VPN is waiting for agent
+ * take also connecting state into account.
+ */
+ connected = provider_is_connected_or_connecting(data);
if (g_str_equal(data->state, "ready")) {
state = CONNMAN_PROVIDER_STATE_READY;
@@ -1075,7 +1086,7 @@ static int provider_disconnect(struct connman_provider *provider)
if (!data)
return -EINVAL;
- if (provider_is_connected(data))
+ if (provider_is_connected_or_connecting(data))
err = disconnect_provider(data);
if (data->call) {
@@ -1729,7 +1740,7 @@ static void destroy_provider(struct connection_data *data)
{
DBG("data %p", data);
- if (provider_is_connected(data))
+ if (provider_is_connected_or_connecting(data))
connman_provider_disconnect(data->provider);
connman_provider_set_data(data->provider, NULL);
@@ -2182,7 +2193,7 @@ static bool vpn_is_valid_transport(struct connman_service *transport)
static void vpn_disconnect_check_provider(struct connection_data *data)
{
- if (provider_is_connected(data)) {
+ if (provider_is_connected_or_connecting(data)) {
/* With NULL service ident NULL is returned immediately */
struct connman_service *service =
connman_service_lookup_from_identifier
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 09/45] vpn: Add VPN agent use callback for plugins
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (7 preceding siblings ...)
2025-07-11 14:26 ` [PATCH 08/45] vpn: Check if connecting when setting state or disconnecting Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-08-01 19:39 ` Denis Kenzior
2025-07-11 14:27 ` [PATCH 10/45] vpn-provider: Transition to CONNECT state with agentless VPNs Jussi Laakkonen
` (38 subsequent siblings)
47 siblings, 1 reply; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Add callback that can be used by the VPN plugins to tell the vpn_driver
whether it uses VPN agent or not. Default to using VPN agent if the
function is not defined.
This is done to accommodate the state transition in vpn-provider when
the VPN does not utilize VPN agent.
---
vpn/plugins/vpn.c | 22 ++++++++++++++++++++++
vpn/plugins/vpn.h | 1 +
vpn/vpn-provider.h | 1 +
3 files changed, 24 insertions(+)
diff --git a/vpn/plugins/vpn.c b/vpn/plugins/vpn.c
index 52f7cac7..2f87cf04 100644
--- a/vpn/plugins/vpn.c
+++ b/vpn/plugins/vpn.c
@@ -800,6 +800,27 @@ static int vpn_route_env_parse(struct vpn_provider *provider, const char *key,
return 0;
}
+static bool vpn_uses_vpn_agent(struct vpn_provider *provider)
+{
+ struct vpn_driver_data *vpn_driver_data = NULL;
+ const char *name = NULL;
+
+ if (!provider)
+ return false;
+
+ name = vpn_provider_get_driver_name(provider);
+ vpn_driver_data = g_hash_table_lookup(driver_hash, name);
+
+ if (vpn_driver_data && vpn_driver_data->vpn_driver->uses_vpn_agent)
+ return vpn_driver_data->vpn_driver->uses_vpn_agent(provider);
+
+ /*
+ * Default to using the VPN agent, in cases where the function is not
+ * implemented. The use of VPN agent must be explicitly dropped.
+ */
+ return true;
+}
+
int vpn_register(const char *name, const struct vpn_driver *vpn_driver,
const char *program)
{
@@ -825,6 +846,7 @@ int vpn_register(const char *name, const struct vpn_driver *vpn_driver,
data->provider_driver.save = vpn_save;
data->provider_driver.set_state = vpn_set_state;
data->provider_driver.route_env_parse = vpn_route_env_parse;
+ data->provider_driver.uses_vpn_agent = vpn_uses_vpn_agent;
if (!driver_hash)
driver_hash = g_hash_table_new_full(g_str_hash,
diff --git a/vpn/plugins/vpn.h b/vpn/plugins/vpn.h
index a8d24fc3..b24cbf9b 100644
--- a/vpn/plugins/vpn.h
+++ b/vpn/plugins/vpn.h
@@ -57,6 +57,7 @@ struct vpn_driver {
int (*route_env_parse) (struct vpn_provider *provider, const char *key,
int *family, unsigned long *idx,
enum vpn_provider_route_type *type);
+ bool (*uses_vpn_agent) (struct vpn_provider *provider);
};
int vpn_register(const char *name, const struct vpn_driver *driver,
diff --git a/vpn/vpn-provider.h b/vpn/vpn-provider.h
index c81476c6..8a8b6bfd 100644
--- a/vpn/vpn-provider.h
+++ b/vpn/vpn-provider.h
@@ -167,6 +167,7 @@ struct vpn_provider_driver {
int (*route_env_parse) (struct vpn_provider *provider, const char *key,
int *family, unsigned long *idx,
enum vpn_provider_route_type *type);
+ bool (*uses_vpn_agent) (struct vpn_provider *provider);
};
int vpn_provider_driver_register(struct vpn_provider_driver *driver);
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 09/45] vpn: Add VPN agent use callback for plugins
2025-07-11 14:27 ` [PATCH 09/45] vpn: Add VPN agent use callback for plugins Jussi Laakkonen
@ 2025-08-01 19:39 ` Denis Kenzior
2025-08-08 12:28 ` Jussi Laakkonen
0 siblings, 1 reply; 69+ messages in thread
From: Denis Kenzior @ 2025-08-01 19:39 UTC (permalink / raw)
To: Jussi Laakkonen, connman
Hi Jussi,
On 7/11/25 9:27 AM, Jussi Laakkonen wrote:
> Add callback that can be used by the VPN plugins to tell the vpn_driver
> whether it uses VPN agent or not. Default to using VPN agent if the
> function is not defined.
>
> This is done to accommodate the state transition in vpn-provider when
> the VPN does not utilize VPN agent.
What about the case where previous credentials are cached?
> ---
> vpn/plugins/vpn.c | 22 ++++++++++++++++++++++
> vpn/plugins/vpn.h | 1 +
> vpn/vpn-provider.h | 1 +
> 3 files changed, 24 insertions(+)
>
<snip>
> diff --git a/vpn/plugins/vpn.h b/vpn/plugins/vpn.h
> index a8d24fc3..b24cbf9b 100644
> --- a/vpn/plugins/vpn.h
> +++ b/vpn/plugins/vpn.h
> @@ -57,6 +57,7 @@ struct vpn_driver {
> int (*route_env_parse) (struct vpn_provider *provider, const char *key,
> int *family, unsigned long *idx,
> enum vpn_provider_route_type *type);
> + bool (*uses_vpn_agent) (struct vpn_provider *provider);
The only user of this would seem to be wireguard which blankly returns false.
Can this be made into a boolean of a set of flags instead?
> };
>
> int vpn_register(const char *name, const struct vpn_driver *driver,
Regards,
-Denis
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 09/45] vpn: Add VPN agent use callback for plugins
2025-08-01 19:39 ` Denis Kenzior
@ 2025-08-08 12:28 ` Jussi Laakkonen
2025-08-08 15:57 ` Denis Kenzior
0 siblings, 1 reply; 69+ messages in thread
From: Jussi Laakkonen @ 2025-08-08 12:28 UTC (permalink / raw)
To: Denis Kenzior, connman
Hi Denis,
On 8/1/25 22:39, Denis Kenzior wrote:
> Hi Jussi,
>
> On 7/11/25 9:27 AM, Jussi Laakkonen wrote:
>> Add callback that can be used by the VPN plugins to tell the vpn_driver
>> whether it uses VPN agent or not. Default to using VPN agent if the
>> function is not defined.
>>
>> This is done to accommodate the state transition in vpn-provider when
>> the VPN does not utilize VPN agent.
>
> What about the case where previous credentials are cached?
This does not have anything to do with caching of credentials? It just
makes it possible to tell if the plugin uses or does not use agent. If
there are issues with caching it should be addressed elsewhere and in
separate patch I think. Sorry for not really understanding what you mean
now.
>
>> ---
>> vpn/plugins/vpn.c | 22 ++++++++++++++++++++++
>> vpn/plugins/vpn.h | 1 +
>> vpn/vpn-provider.h | 1 +
>> 3 files changed, 24 insertions(+)
>>
>
> <snip>
>
>> diff --git a/vpn/plugins/vpn.h b/vpn/plugins/vpn.h
>> index a8d24fc3..b24cbf9b 100644
>> --- a/vpn/plugins/vpn.h
>> +++ b/vpn/plugins/vpn.h
>> @@ -57,6 +57,7 @@ struct vpn_driver {
>> int (*route_env_parse) (struct vpn_provider *provider, const
>> char *key,
>> int *family, unsigned long *idx,
>> enum vpn_provider_route_type *type);
>> + bool (*uses_vpn_agent) (struct vpn_provider *provider);
>
> The only user of this would seem to be wireguard which blankly returns
> false. Can this be made into a boolean of a set of flags instead?
I simply followed the approach that is used by the VPNs in general.
Seemed simplest and most readable way to do it.
Sure, it can be extended to be a setup flag based system but it is then
different of what the VPNs generally use, possibly requiring more changes.
>
>> };
>> int vpn_register(const char *name, const struct vpn_driver *driver,
>
> Regards,
> -Denis
BR,
Jussi
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 09/45] vpn: Add VPN agent use callback for plugins
2025-08-08 12:28 ` Jussi Laakkonen
@ 2025-08-08 15:57 ` Denis Kenzior
2025-08-13 9:22 ` Jussi Laakkonen
2025-08-13 11:20 ` Jussi Laakkonen
0 siblings, 2 replies; 69+ messages in thread
From: Denis Kenzior @ 2025-08-08 15:57 UTC (permalink / raw)
To: Jussi Laakkonen, connman
Hi Jussi,
On 8/8/25 7:28 AM, Jussi Laakkonen wrote:
> Hi Denis,
>
> On 8/1/25 22:39, Denis Kenzior wrote:
>> Hi Jussi,
>>
>> On 7/11/25 9:27 AM, Jussi Laakkonen wrote:
>>> Add callback that can be used by the VPN plugins to tell the vpn_driver
>>> whether it uses VPN agent or not. Default to using VPN agent if the
>>> function is not defined.
>>>
>>> This is done to accommodate the state transition in vpn-provider when
>>> the VPN does not utilize VPN agent.
>>
>> What about the case where previous credentials are cached?
>
> This does not have anything to do with caching of credentials? It just makes it
> possible to tell if the plugin uses or does not use agent. If there are issues
> with caching it should be addressed elsewhere and in separate patch I think.
> Sorry for not really understanding what you mean now.
So maybe I didn't fully follow the logic, so apologies if that's the case. By
my reading we have two possibilities:
1. driver has to ask for credentials. So when .connect is invoked, core
automatically puts the state into 'Association' and lets the driver report back
when the credentials are obtained.
2. driver will never ask for credentials. Core automatically puts the state
into 'Association' and then 'Connecting' or whatever.
Are there situations where the driver might cache the credentials and not ask
for them again on subsequent connects? The core could then treat this as case 2
above.
<snip>
>>
>> The only user of this would seem to be wireguard which blankly returns false.
>> Can this be made into a boolean of a set of flags instead?
>
> I simply followed the approach that is used by the VPNs in general. Seemed
> simplest and most readable way to do it.
Hmm, but we already have:
#define VPN_FLAG_NO_TUN 1
#define VPN_FLAG_NO_DAEMON 2
Wouldn't this fit the existing pattern?
Regards,
-Denis
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 09/45] vpn: Add VPN agent use callback for plugins
2025-08-08 15:57 ` Denis Kenzior
@ 2025-08-13 9:22 ` Jussi Laakkonen
2025-08-13 11:20 ` Jussi Laakkonen
1 sibling, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-08-13 9:22 UTC (permalink / raw)
To: Denis Kenzior, connman
Hi Denis,
On 8/8/25 18:57, Denis Kenzior wrote:
> Hi Jussi,
>
> On 8/8/25 7:28 AM, Jussi Laakkonen wrote:
>> Hi Denis,
>>
>> On 8/1/25 22:39, Denis Kenzior wrote:
>>> Hi Jussi,
>>>
>>> On 7/11/25 9:27 AM, Jussi Laakkonen wrote:
>>>> Add callback that can be used by the VPN plugins to tell the vpn_driver
>>>> whether it uses VPN agent or not. Default to using VPN agent if the
>>>> function is not defined.
>>>>
>>>> This is done to accommodate the state transition in vpn-provider when
>>>> the VPN does not utilize VPN agent.
>>>
>>> What about the case where previous credentials are cached?
>>
>> This does not have anything to do with caching of credentials? It just
>> makes it possible to tell if the plugin uses or does not use agent. If
>> there are issues with caching it should be addressed elsewhere and in
>> separate patch I think. Sorry for not really understanding what you
>> mean now.
>
> So maybe I didn't fully follow the logic, so apologies if that's the
> case. By my reading we have two possibilities:
>
> 1. driver has to ask for credentials. So when .connect is invoked, core
> automatically puts the state into 'Association' and lets the driver
> report back when the credentials are obtained.
> 2. driver will never ask for credentials. Core automatically puts the
> state into 'Association' and then 'Connecting' or whatever.
>
> Are there situations where the driver might cache the credentials and
> not ask for them again on subsequent connects? The core could then
> treat this as case 2 above.
The configuration for the VPNs are read by vpn-provider.c and that does
not update them when the content changes on the file that was read. That
is why using VPN agent is a better option as that can be made to handle
the caching cases well, and the errors are reported back to the agent so
it can clear them when they're invalid. This is how it works on Sailfish
OS, so an external storage is used for them by the agent to make sure
caching is not an issue. Thus, the implementation here should not take a
stance on it.
Since WireGuard does get the credentials through vpn-provider.c I think
this goes beyond this patch set and would require further work to
address any caching issues. It is already blowing out of proportions now
I think.
>
> <snip>
>
>>>
>>> The only user of this would seem to be wireguard which blankly
>>> returns false. Can this be made into a boolean of a set of flags
>>> instead?
>>
>> I simply followed the approach that is used by the VPNs in general.
>> Seemed simplest and most readable way to do it.
>
> Hmm, but we already have:
>
> #define VPN_FLAG_NO_TUN 1
> #define VPN_FLAG_NO_DAEMON 2
>
> Wouldn't this fit the existing pattern?
>
Um, no. A daemonless VPN can have a VPN agent in use. I was planning to
implement this for WireGuard too. But for now this minor change is a
clear distinction between the cases of agent and no-agent from my
perspective.
Br,
Jussi
P.S. I'll send a v2 of this whole set as some changes required changing
the commits here and there and I lost track of them, easier to put a new
complete one to avoid problems.
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 09/45] vpn: Add VPN agent use callback for plugins
2025-08-08 15:57 ` Denis Kenzior
2025-08-13 9:22 ` Jussi Laakkonen
@ 2025-08-13 11:20 ` Jussi Laakkonen
1 sibling, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-08-13 11:20 UTC (permalink / raw)
To: Denis Kenzior, connman
Hi Denis,
On 8/8/25 18:57, Denis Kenzior wrote:
> Hi Jussi,
>
> On 8/8/25 7:28 AM, Jussi Laakkonen wrote:
>
> Hmm, but we already have:
>
> #define VPN_FLAG_NO_TUN 1
> #define VPN_FLAG_NO_DAEMON 2
>
> Wouldn't this fit the existing pattern?
>
Ah, now I get what you meant. To extend these flags. Well, that should
be doable here.
BR,
Jussi
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 10/45] vpn-provider: Transition to CONNECT state with agentless VPNs
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (8 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 09/45] vpn: Add VPN agent use callback for plugins Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 11/45] doc: Update VPN documentation for association state Jussi Laakkonen
` (37 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Set to transition to CONNECT state immediately after ASSOCIATION state
after initializing connection procedure with a VPN if it does not use
VPN agent. This is done to accommodate the full state machine
transitions as the VPN agent, when success, will do the transition but
when VPN agent is not used the transition would be required to be done
by the plugin.
---
vpn/vpn-provider.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index 56040e65..b21e9e61 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -1653,8 +1653,19 @@ int __vpn_provider_connect(struct vpn_provider *provider, DBusMessage *msg)
} else
return -EOPNOTSUPP;
- if (err == -EINPROGRESS)
- vpn_provider_set_state(provider, VPN_PROVIDER_STATE_ASSOCIATION);
+ if (err == -EINPROGRESS) {
+ vpn_provider_set_state(provider,
+ VPN_PROVIDER_STATE_ASSOCIATION);
+
+ /*
+ * If the VPN does not use VPN agent do direct transition to
+ * connect in order to support the complete state machine.
+ */
+ if (provider->driver && provider->driver->uses_vpn_agent &&
+ !provider->driver->uses_vpn_agent(provider))
+ vpn_provider_set_state(provider,
+ VPN_PROVIDER_STATE_CONNECT);
+ }
return err;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 11/45] doc: Update VPN documentation for association state
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (9 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 10/45] vpn-provider: Transition to CONNECT state with agentless VPNs Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 12/45] wireguard: Add saving of provider properties Jussi Laakkonen
` (36 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Add brief descriptions of the association state. Add it to parameter
descriptions as well.
---
doc/vpn-connection-api.txt | 4 ++--
doc/vpn-overview.txt | 7 ++++++-
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/doc/vpn-connection-api.txt b/doc/vpn-connection-api.txt
index 2d3e0078..df070957 100644
--- a/doc/vpn-connection-api.txt
+++ b/doc/vpn-connection-api.txt
@@ -104,8 +104,8 @@ Properties string State [readonly]
The connection state information.
- Valid states are "idle", "failure", "configuration",
- "ready", "disconnect".
+ Valid states are "idle", "failure", "association",
+ "configuration", "ready", "disconnect".
string Type [readonly]
diff --git a/doc/vpn-overview.txt b/doc/vpn-overview.txt
index d2d14a0c..74f5695e 100644
--- a/doc/vpn-overview.txt
+++ b/doc/vpn-overview.txt
@@ -66,7 +66,12 @@ VPN agent interface described in vpn-agent-api.txt is used for
interaction between the connectivity UI and ConnMan. A VPN agent
registered via Management interface gets requests from the VPN plugins
to input credentials or other authentication information for the VPN
-connection and offers information about the VPN to be connected.
+connection and offers information about the VPN to be connected. When
+waiting for input via VPN agent the state of the VPN is "association"
+and after getting the input the state transitions to "connect". If the
+VPN does not wish to use VPN agent this can be explicitly defined by
+implementing "uses_vpn_agent()" returning "false" indicating that the
+state is transitioned to "connect" when connecting the VPN.
In addition to basic credentials, there are additional types of optional
and control parameters. The user can dictate whether to store the
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 12/45] wireguard: Add saving of provider properties
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (10 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 11/45] doc: Update VPN documentation for association state Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 13/45] wireguard: Use positive errors for VPN provider connect_cb Jussi Laakkonen
` (35 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Save all provider properties to provider configuration file. This
follows the example defined in doc/vpn-config-format.txt
---
vpn/plugins/wireguard.c | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index 03658943..735bac58 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -50,6 +50,7 @@
#include "wireguard.h"
#define DNS_RERESOLVE_TIMEOUT 20
+#define ARRAY_SIZE(a) (sizeof(a)/sizeof(a[0]))
struct wireguard_info {
struct wg_device device;
@@ -67,6 +68,21 @@ struct sockaddr_u {
};
};
+struct {
+ const char *opt;
+ bool save;
+} wg_options[] = {
+ {"WireGuard.Address", true},
+ {"WireGuard.ListenPort", true},
+ {"WireGuard.DNS", true},
+ {"WireGuard.PrivateKey", true}, // TODO set false after agent support
+ {"WireGuard.PresharedKey", true}, // TODO set false after agent support
+ {"WireGuard.PublicKey", true},
+ {"WireGuard.AllowedIPs", true},
+ {"WireGuard.EndpointPort", true},
+ {"WireGuard.PersistentKeepalive", true}
+};
+
static int parse_key(const char *str, wg_key key)
{
unsigned char *buf;
@@ -462,10 +478,32 @@ static void wg_disconnect(struct vpn_provider *provider)
g_free(info);
}
+static int wg_save(struct vpn_provider *provider, GKeyFile *keyfile)
+{
+ const char *option;
+ int i;
+
+ for (i = 0; i < (int)ARRAY_SIZE(wg_options); i++) {
+ if (!wg_options[i].save)
+ continue;
+
+ option = vpn_provider_get_string(provider, wg_options[i].opt);
+ if (!option)
+ continue;
+
+ g_key_file_set_string(keyfile,
+ vpn_provider_get_save_group(provider),
+ wg_options[i].opt, option);
+ }
+
+ return 0;
+}
+
static struct vpn_driver vpn_driver = {
.flags = VPN_FLAG_NO_TUN | VPN_FLAG_NO_DAEMON,
.connect = wg_connect,
.disconnect = wg_disconnect,
+ .save = wg_save,
};
static int wg_init(void)
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 13/45] wireguard: Use positive errors for VPN provider connect_cb
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (11 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 12/45] wireguard: Add saving of provider properties Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 14/45] vpn: Fix VPN_FLAG_NO_DAEMON use in error cases Jussi Laakkonen
` (34 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
The vpn-provider.c:connect_cb() expects the errors to be positive ints.
Remove the sign when calling the cb.
And add debug to see what is really causing the error. For example, when
parsing fails.
---
vpn/plugins/wireguard.c | 30 ++++++++++++++++++++++--------
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index 735bac58..de57b15f 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -349,8 +349,10 @@ static int wg_connect(struct vpn_provider *provider,
option = vpn_provider_get_string(provider, "WireGuard.DNS");
if (option) {
err = vpn_provider_set_nameservers(provider, option);
- if (err)
+ if (err) {
+ DBG("Cannot set nameservers %s", option);
goto done;
+ }
}
option = vpn_provider_get_string(provider, "WireGuard.PrivateKey");
@@ -359,8 +361,10 @@ static int wg_connect(struct vpn_provider *provider,
goto done;
}
err = parse_key(option, info->device.private_key);
- if (err)
+ if (err) {
+ DBG("Failed to parse private key");
goto done;
+ }
option = vpn_provider_get_string(provider, "WireGuard.PublicKey");
if (!option) {
@@ -368,15 +372,19 @@ static int wg_connect(struct vpn_provider *provider,
goto done;
}
err = parse_key(option, info->peer.public_key);
- if (err)
+ if (err) {
+ DBG("Failed to parse public key");
goto done;
+ }
option = vpn_provider_get_string(provider, "WireGuard.PresharedKey");
if (option) {
info->peer.flags |= WGPEER_HAS_PRESHARED_KEY;
err = parse_key(option, info->peer.preshared_key);
- if (err)
+ if (err) {
+ DBG("Failed to parse pre-shared key");
goto done;
+ }
}
option = vpn_provider_get_string(provider, "WireGuard.AllowedIPs");
@@ -385,8 +393,10 @@ static int wg_connect(struct vpn_provider *provider,
goto done;
}
err = parse_allowed_ips(option, &info->peer);
- if (err)
+ if (err) {
+ DBG("Failed to parse allowed IPs %s", option);
goto done;
+ }
option = vpn_provider_get_string(provider,
"WireGuard.PersistentKeepalive");
@@ -404,8 +414,10 @@ static int wg_connect(struct vpn_provider *provider,
gateway = vpn_provider_get_string(provider, "Host");
err = parse_endpoint(gateway, option,
(struct sockaddr_u *)&info->peer.endpoint.addr);
- if (err)
+ if (err) {
+ DBG("Failed to parse endpoint %s gateway %s", option, gateway);
goto done;
+ }
info->endpoint_fqdn = g_strdup(gateway);
info->port = g_strdup(option);
@@ -416,8 +428,10 @@ static int wg_connect(struct vpn_provider *provider,
goto done;
}
err = parse_address(option, gateway, &ipaddress);
- if (err)
+ if (err) {
+ DBG("Failed to parse address %s gateway %s", option, gateway);
goto done;
+ }
ifname = get_ifname();
if (!ifname) {
@@ -446,7 +460,7 @@ static int wg_connect(struct vpn_provider *provider,
done:
if (cb)
- cb(provider, user_data, err);
+ cb(provider, user_data, -err);
connman_ipaddress_free(ipaddress);
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 14/45] vpn: Fix VPN_FLAG_NO_DAEMON use in error cases
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (12 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 13/45] wireguard: Use positive errors for VPN provider connect_cb Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 15/45] wireguard: Handle disconnect, error and network errors better Jussi Laakkonen
` (33 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
The VPNs that have no daemon, WireGuard, should not be called to
disconnect twice because in error situations the change of state from
error case is overridden. This makes it impossible to track the state
via an UI that relies on state notifications and uses autoconnect to
toggle active VPN(s).
Make vpn_died VPN_FLAG_NO_DAEMON friendly that can be called also by the
VPNs without a task. This makes it possible to use vpn_died in WireGuard
as final call to make state transitions complete.
Make the state transitions of VPN_FLAG_NO_DAEMON to use the
VPN_STATE_ASSOCIATION properly. This is the initial state when
connecting, and the VPN_STATE_CONNECT is set by the
update_provider_state() when driver replies that connection is ok.
Also cleanup some code to use the same base more.
---
vpn/plugins/vpn.c | 57 ++++++++++++++++++++++++++---------------------
1 file changed, 32 insertions(+), 25 deletions(-)
diff --git a/vpn/plugins/vpn.c b/vpn/plugins/vpn.c
index 2f87cf04..81a62ff4 100644
--- a/vpn/plugins/vpn.c
+++ b/vpn/plugins/vpn.c
@@ -91,7 +91,13 @@ static int stop_vpn(struct vpn_provider *provider)
if (vpn_driver_data && vpn_driver_data->vpn_driver &&
vpn_driver_data->vpn_driver->flags & VPN_FLAG_NO_TUN) {
- vpn_driver_data->vpn_driver->disconnect(data->provider);
+ /*
+ * Disconnect only VPNs with daemon, otherwise in error return
+ * there is a double free with vpn_died() or the failure state
+ * is overridden by changes made by disconnect to state.
+ */
+ if (!(vpn_driver_data->vpn_driver->flags & VPN_FLAG_NO_DAEMON))
+ vpn_driver_data->vpn_driver->disconnect(data->provider);
return 0;
}
@@ -131,16 +137,27 @@ void vpn_died(struct connman_task *task, int exit_code, void *user_data)
{
struct vpn_provider *provider = user_data;
struct vpn_data *data = vpn_provider_get_data(provider);
+ struct vpn_driver_data *vpn_data = NULL;
+ const char *name;
int state = VPN_STATE_FAILURE;
enum vpn_provider_error ret;
+ bool no_daemon = false;
+
+ name = vpn_provider_get_driver_name(provider);
+ if (name) {
+ vpn_data = g_hash_table_lookup(driver_hash, name);
+ if (vpn_data && vpn_data->vpn_driver)
+ no_daemon = vpn_data->vpn_driver->flags &
+ VPN_FLAG_NO_DAEMON;
+ }
- DBG("provider %p data %p", provider, data);
+ DBG("provider %p data %p no_daemon %d", provider, data, no_daemon);
if (!data)
goto vpn_exit;
/* The task may die after we have already started the new one */
- if (data->task != task)
+ if (!no_daemon && data->task != task)
goto done;
state = data->state;
@@ -156,15 +173,7 @@ void vpn_died(struct connman_task *task, int exit_code, void *user_data)
vpn_exit:
if (state != VPN_STATE_READY && state != VPN_STATE_DISCONNECT) {
- const char *name;
- struct vpn_driver_data *vpn_data = NULL;
-
- name = vpn_provider_get_driver_name(provider);
- if (name)
- vpn_data = g_hash_table_lookup(driver_hash, name);
-
- if (vpn_data && vpn_data->vpn_driver &&
- vpn_data->vpn_driver->error_code)
+ if (vpn_data && vpn_data->vpn_driver->error_code)
ret = vpn_data->vpn_driver->error_code(provider,
exit_code);
else
@@ -183,7 +192,8 @@ vpn_exit:
}
done:
- connman_task_destroy(task);
+ if (!no_daemon)
+ connman_task_destroy(task);
}
int vpn_set_ifname(struct vpn_provider *provider, const char *ifname)
@@ -546,6 +556,8 @@ static gboolean update_provider_state(gpointer data)
vpn_data->watch = vpn_rtnl_add_newlink_watch(index,
vpn_newlink, provider);
connman_inet_ifup(index);
+ vpn_data->state = VPN_STATE_CONNECT;
+ vpn_provider_set_state(provider, VPN_PROVIDER_STATE_CONNECT);
return FALSE;
}
@@ -621,18 +633,10 @@ static int vpn_connect(struct vpn_provider *provider,
ret = vpn_driver_data->vpn_driver->connect(provider,
NULL, NULL, cb, dbus_sender, user_data);
- if (ret) {
- stop_vpn(provider);
- goto exist_err;
- }
+ if (!ret)
+ g_timeout_add(1, update_provider_state, provider);
- DBG("%s started with dev %s",
- vpn_driver_data->provider_driver.name, data->if_name);
-
- data->state = VPN_STATE_CONNECT;
-
- g_timeout_add(1, update_provider_state, provider);
- return -EINPROGRESS;
+ goto done;
}
vpn_plugin_data =
@@ -658,9 +662,12 @@ static int vpn_connect(struct vpn_provider *provider,
ret = vpn_driver_data->vpn_driver->connect(provider, data->task,
data->if_name, cb, dbus_sender,
user_data);
+
+done:
if (ret < 0 && ret != -EINPROGRESS) {
stop_vpn(provider);
- connman_task_destroy(data->task);
+ if (data->task)
+ connman_task_destroy(data->task);
data->task = NULL;
goto exist_err;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 15/45] wireguard: Handle disconnect, error and network errors better
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (13 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 14/45] vpn: Fix VPN_FLAG_NO_DAEMON use in error cases Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 16/45] gresolv: Add generic error for GResolv struct with getter Jussi Laakkonen
` (32 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Make the plugin to handle disconnect better by setting state, and
calling vpn_died() with the exit code to make the states indicated
properly to higher layers. This currently handles the ENODEV as an error
code transformation to 0, since it can happen when the device is not up
yet.
When the parameters are wrong use login failed error with ECONNABORTED
to indicate that autoconnect should be stopped on this VPN. It also,
with other changes to vpn.c, makes it possible to keep the error for
higher layers.
Add a limit for the DNS reresolve attempts that makes WireGuard die
after the limit is reached. This is for the cases when the IP
configuration is wrong on the client and after connecting the endpoint
cannot be resolved, and should be indicated as an error to higher
layers. Also, in case the endpoint cannot be resolved, because of
getaddrinfo()'s lengthy timeout vpnd becomes inoperable during that
period. It performs a bit better when the timeout is recreated after
every cb call, but not perfect. It should be running in a thread because
of this deadlock but this is the first step on improving this with
invalid configurations.
---
vpn/plugins/wireguard.c | 115 +++++++++++++++++++++++++++++++---------
1 file changed, 91 insertions(+), 24 deletions(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index de57b15f..2c5981f8 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -50,9 +50,11 @@
#include "wireguard.h"
#define DNS_RERESOLVE_TIMEOUT 20
+#define DNS_RERESOLVE_ERROR_LIMIT 5
#define ARRAY_SIZE(a) (sizeof(a)/sizeof(a[0]))
struct wireguard_info {
+ struct vpn_provider *provider;
struct wg_device device;
struct wg_peer peer;
char *endpoint_fqdn;
@@ -155,6 +157,7 @@ static int parse_endpoint(const char *host, const char *port, struct sockaddr_u
struct addrinfo hints;
struct addrinfo *result, *rp;
int sk;
+ int err;
memset(&hints, 0, sizeof(struct addrinfo));
hints.ai_family = AF_UNSPEC;
@@ -162,8 +165,9 @@ static int parse_endpoint(const char *host, const char *port, struct sockaddr_u
hints.ai_flags = 0;
hints.ai_protocol = 0;
- if (getaddrinfo(host, port, &hints, &result) < 0) {
- DBG("Failed to resolve host address");
+ err = getaddrinfo(host, port, &hints, &result);
+ if (err < 0) {
+ DBG("Failed to resolve host address: %s", gai_strerror(err));
return -EINVAL;
}
@@ -287,22 +291,38 @@ static bool sockaddr_cmp_addr(struct sockaddr_u *a, struct sockaddr_u *b)
return false;
}
+static void run_dns_reresolve(struct wireguard_info *info);
+
static gboolean wg_dns_reresolve_cb(gpointer user_data)
{
struct wireguard_info *info = user_data;
struct sockaddr_u addr;
int err;
- DBG("");
+ if (vpn_provider_get_connection_errors(info->provider) >=
+ DNS_RERESOLVE_ERROR_LIMIT) {
+ connman_warn("reresolve error limit reached");
+ vpn_died(NULL, -ENONET, info->provider);
+ return G_SOURCE_REMOVE;
+ }
+ /* If this fails after being connected it means configuration error
+ * that results in connection errors. */
err = parse_endpoint(info->endpoint_fqdn,
info->port, &addr);
- if (err)
- return TRUE;
+ if (err) {
+ if (info->provider)
+ vpn_provider_add_error(info->provider,
+ VPN_PROVIDER_ERROR_CONNECT_FAILED);
+ run_dns_reresolve(info);
+ return G_SOURCE_REMOVE;
+ }
if (sockaddr_cmp_addr(&addr,
- (struct sockaddr_u *)&info->peer.endpoint.addr))
- return TRUE;
+ (struct sockaddr_u *)&info->peer.endpoint.addr)) {
+ run_dns_reresolve(info);
+ return G_SOURCE_REMOVE;
+ }
if (addr.sa.sa_family == AF_INET)
memcpy(&info->peer.endpoint.addr, &addr.sin,
@@ -317,7 +337,17 @@ static gboolean wg_dns_reresolve_cb(gpointer user_data)
DBG("Failed to update Endpoint address for WireGuard device %s",
info->device.name);
- return TRUE;
+ run_dns_reresolve(info);
+ return G_SOURCE_REMOVE;
+}
+
+static void run_dns_reresolve(struct wireguard_info *info)
+{
+ if (info->reresolve_id)
+ g_source_remove(info->reresolve_id);
+
+ info->reresolve_id = g_timeout_add_seconds(DNS_RERESOLVE_TIMEOUT,
+ wg_dns_reresolve_cb, info);
}
static int wg_connect(struct vpn_provider *provider,
@@ -336,8 +366,12 @@ static int wg_connect(struct vpn_provider *provider,
info->device.flags = WGDEVICE_HAS_PRIVATE_KEY;
info->device.first_peer = &info->peer;
info->device.last_peer = &info->peer;
+ info->provider = vpn_provider_ref(provider);
+
+ DBG("");
vpn_provider_set_plugin_data(provider, info);
+ vpn_provider_set_auth_error_limit(provider, 1);
option = vpn_provider_get_string(provider, "WireGuard.ListenPort");
if (option) {
@@ -351,30 +385,30 @@ static int wg_connect(struct vpn_provider *provider,
err = vpn_provider_set_nameservers(provider, option);
if (err) {
DBG("Cannot set nameservers %s", option);
- goto done;
+ goto error;
}
}
option = vpn_provider_get_string(provider, "WireGuard.PrivateKey");
if (!option) {
DBG("WireGuard.PrivateKey is missing");
- goto done;
+ goto error;
}
err = parse_key(option, info->device.private_key);
if (err) {
DBG("Failed to parse private key");
- goto done;
+ goto error;
}
option = vpn_provider_get_string(provider, "WireGuard.PublicKey");
if (!option) {
DBG("WireGuard.PublicKey is missing");
- goto done;
+ goto error;
}
err = parse_key(option, info->peer.public_key);
if (err) {
DBG("Failed to parse public key");
- goto done;
+ goto error;
}
option = vpn_provider_get_string(provider, "WireGuard.PresharedKey");
@@ -383,19 +417,19 @@ static int wg_connect(struct vpn_provider *provider,
err = parse_key(option, info->peer.preshared_key);
if (err) {
DBG("Failed to parse pre-shared key");
- goto done;
+ goto error;
}
}
option = vpn_provider_get_string(provider, "WireGuard.AllowedIPs");
if (!option) {
DBG("WireGuard.AllowedIPs is missing");
- goto done;
+ goto error;
}
err = parse_allowed_ips(option, &info->peer);
if (err) {
DBG("Failed to parse allowed IPs %s", option);
- goto done;
+ goto error;
}
option = vpn_provider_get_string(provider,
@@ -415,8 +449,8 @@ static int wg_connect(struct vpn_provider *provider,
err = parse_endpoint(gateway, option,
(struct sockaddr_u *)&info->peer.endpoint.addr);
if (err) {
- DBG("Failed to parse endpoint %s gateway %s", option, gateway);
- goto done;
+ DBG("Failed to parse endpoint %s:%s", gateway, option);
+ goto error;
}
info->endpoint_fqdn = g_strdup(gateway);
@@ -425,12 +459,12 @@ static int wg_connect(struct vpn_provider *provider,
option = vpn_provider_get_string(provider, "WireGuard.Address");
if (!option) {
DBG("Missing WireGuard.Address configuration");
- goto done;
+ goto error;
}
err = parse_address(option, gateway, &ipaddress);
if (err) {
DBG("Failed to parse address %s gateway %s", option, gateway);
- goto done;
+ goto error;
}
ifname = get_ifname();
@@ -465,16 +499,27 @@ done:
connman_ipaddress_free(ipaddress);
if (!err)
- info->reresolve_id =
- g_timeout_add_seconds(DNS_RERESOLVE_TIMEOUT,
- wg_dns_reresolve_cb, info);
+ run_dns_reresolve(info);
return err;
+
+error:
+ /*
+ * TODO: add own category for parameter errors. This is to avoid
+ * looping when parameters are incorrect and VPN stays in failed
+ * state.
+ */
+ vpn_provider_add_error(provider, VPN_PROVIDER_ERROR_LOGIN_FAILED);
+ err = -ECONNABORTED;
+ goto done;
}
static void wg_disconnect(struct vpn_provider *provider)
{
struct wireguard_info *info;
+ int exit_code;
+
+ DBG("");
info = vpn_provider_get_plugin_data(provider);
if (!info)
@@ -485,11 +530,32 @@ static void wg_disconnect(struct vpn_provider *provider)
vpn_provider_set_plugin_data(provider, NULL);
- wg_del_device(info->device.name);
+ vpn_provider_set_state(provider, VPN_PROVIDER_STATE_DISCONNECT);
+
+ exit_code = wg_del_device(info->device.name);
+ vpn_provider_unref(info->provider);
g_free(info->endpoint_fqdn);
g_free(info->port);
g_free(info);
+
+ DBG("exiting with %d", exit_code);
+
+ /* No task for no daemon VPN - use VPN died with no task. */
+ vpn_died(NULL, exit_code, provider);
+}
+
+static int wg_error_code(struct vpn_provider *provider, int exit_code)
+{
+ DBG("exit_code %d", exit_code);
+
+ switch (exit_code) {
+ /* Failed to parse configuration -> wg_del_device() has no to delete */
+ case -ENODEV:
+ return 0;
+ default:
+ return exit_code;
+ }
}
static int wg_save(struct vpn_provider *provider, GKeyFile *keyfile)
@@ -518,6 +584,7 @@ static struct vpn_driver vpn_driver = {
.connect = wg_connect,
.disconnect = wg_disconnect,
.save = wg_save,
+ .error_code = wg_error_code
};
static int wg_init(void)
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 16/45] gresolv: Add generic error for GResolv struct with getter
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (14 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 15/45] wireguard: Handle disconnect, error and network errors better Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-08-04 14:37 ` Denis Kenzior
2025-07-11 14:27 ` [PATCH 17/45] vpn-util: Add wrappers for GResolv hostname lookup use Jussi Laakkonen
` (31 subsequent siblings)
47 siblings, 1 reply; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Add a error for the struct to be used with hostname lookup. The function
should return either 0 in case of error or the source id, this fixes the
wrong returns when error happens and makes it possible to retrieve the
error afterwards.
---
gweb/gresolv.c | 15 +++++++++++++--
gweb/gresolv.h | 2 ++
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/gweb/gresolv.c b/gweb/gresolv.c
index 8101d718..1f1db474 100644
--- a/gweb/gresolv.c
+++ b/gweb/gresolv.c
@@ -116,6 +116,7 @@ struct _GResolv {
GResolvDebugFunc debug_func;
gpointer debug_data;
+ int err;
};
#define debug(resolv, format, arg...) \
@@ -1060,7 +1061,8 @@ guint g_resolv_lookup_hostname(GResolv *resolv, const char *hostname,
if (resolv->result_family != AF_INET6) {
if (add_query(lookup, hostname, ns_t_a)) {
g_free(lookup);
- return -EIO;
+ resolv->err = -EIO;
+ return 0;
}
}
@@ -1073,7 +1075,8 @@ guint g_resolv_lookup_hostname(GResolv *resolv, const char *hostname,
}
g_free(lookup);
- return -EIO;
+ resolv->err = -EIO;
+ return 0;
}
}
@@ -1119,3 +1122,11 @@ bool g_resolv_set_address_family(GResolv *resolv, int family)
return true;
}
+
+int g_resolv_get_error(GResolv *resolv)
+{
+ if (!resolv)
+ return 0;
+
+ return resolv->err;
+}
diff --git a/gweb/gresolv.h b/gweb/gresolv.h
index 5e82c168..1994ac34 100644
--- a/gweb/gresolv.h
+++ b/gweb/gresolv.h
@@ -71,6 +71,8 @@ bool g_resolv_cancel_lookup(GResolv *resolv, guint id);
bool g_resolv_set_address_family(GResolv *resolv, int family);
+int g_resolv_get_error(GResolv *resolv);
+
#ifdef __cplusplus
}
#endif
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 16/45] gresolv: Add generic error for GResolv struct with getter
2025-07-11 14:27 ` [PATCH 16/45] gresolv: Add generic error for GResolv struct with getter Jussi Laakkonen
@ 2025-08-04 14:37 ` Denis Kenzior
2025-08-08 12:34 ` Jussi Laakkonen
0 siblings, 1 reply; 69+ messages in thread
From: Denis Kenzior @ 2025-08-04 14:37 UTC (permalink / raw)
To: Jussi Laakkonen, connman
Hi Jussi,
On 7/11/25 9:27 AM, Jussi Laakkonen wrote:
> Add a error for the struct to be used with hostname lookup. The function
> should return either 0 in case of error or the source id, this fixes the
> wrong returns when error happens and makes it possible to retrieve the
> error afterwards.
> ---
> gweb/gresolv.c | 15 +++++++++++++--
> gweb/gresolv.h | 2 ++
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/gweb/gresolv.c b/gweb/gresolv.c
> index 8101d718..1f1db474 100644
> --- a/gweb/gresolv.c
> +++ b/gweb/gresolv.c
> @@ -116,6 +116,7 @@ struct _GResolv {
>
> GResolvDebugFunc debug_func;
> gpointer debug_data;
> + int err;
These errors are really local to g_resolv_lookup_hostname, so not sure this
warrants a member.
> };
>
> #define debug(resolv, format, arg...) \
> @@ -1060,7 +1061,8 @@ guint g_resolv_lookup_hostname(GResolv *resolv, const char *hostname,
> if (resolv->result_family != AF_INET6) {
> if (add_query(lookup, hostname, ns_t_a)) {
> g_free(lookup);
> - return -EIO;
> + resolv->err = -EIO;
> + return 0;
Since this is a glib library, have you considered returning 0 and setting GError
in/out parameter instead?
> }
> }
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 16/45] gresolv: Add generic error for GResolv struct with getter
2025-08-04 14:37 ` Denis Kenzior
@ 2025-08-08 12:34 ` Jussi Laakkonen
0 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-08-08 12:34 UTC (permalink / raw)
To: Denis Kenzior, connman
Hi Denis,
On 8/4/25 17:37, Denis Kenzior wrote:
> Hi Jussi,
>
> On 7/11/25 9:27 AM, Jussi Laakkonen wrote:
>> Add a error for the struct to be used with hostname lookup. The function
>> should return either 0 in case of error or the source id, this fixes the
>> wrong returns when error happens and makes it possible to retrieve the
>> error afterwards.
>> ---
>> gweb/gresolv.c | 15 +++++++++++++--
>> gweb/gresolv.h | 2 ++
>> 2 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/gweb/gresolv.c b/gweb/gresolv.c
>> index 8101d718..1f1db474 100644
>> --- a/gweb/gresolv.c
>> +++ b/gweb/gresolv.c
>> @@ -116,6 +116,7 @@ struct _GResolv {
>> GResolvDebugFunc debug_func;
>> gpointer debug_data;
>> + int err;
>
> These errors are really local to g_resolv_lookup_hostname, so not sure
> this warrants a member.
>
Personally I think that the error should be in some way retrievable by
the caller. Previously the error and the source id's were mixed which is
never a good option.
>> };
>> #define debug(resolv, format, arg...) \
>> @@ -1060,7 +1061,8 @@ guint g_resolv_lookup_hostname(GResolv *resolv,
>> const char *hostname,
>> if (resolv->result_family != AF_INET6) {
>> if (add_query(lookup, hostname, ns_t_a)) {
>> g_free(lookup);
>> - return -EIO;
>> + resolv->err = -EIO;
>> + return 0;
>
> Since this is a glib library, have you considered returning 0 and
> setting GError in/out parameter instead?
Yeah, sure, I can change it to GError. Just tried to keep things really
simple here.
>
>> }
>> }
>
> Regards,
> -Denis
BR,
Jussi
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 17/45] vpn-util: Add wrappers for GResolv hostname lookup use
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (15 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 16/45] gresolv: Add generic error for GResolv struct with getter Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-08-04 14:39 ` Denis Kenzior
2025-07-11 14:27 ` [PATCH 18/45] wireguard: Use GResolv for DNS reresolve to avoid blocking Jussi Laakkonen
` (30 subsequent siblings)
47 siblings, 1 reply; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
To avoid linking GResolv with every VPN plugin that uses it, add wrappers
for the required calls in VPN utils.
---
vpn/vpn-util.c | 30 ++++++++++++++++++++++++++++++
vpn/vpn.h | 10 ++++++++++
2 files changed, 40 insertions(+)
diff --git a/vpn/vpn-util.c b/vpn/vpn-util.c
index bc3b01dd..2e526701 100644
--- a/vpn/vpn-util.c
+++ b/vpn/vpn-util.c
@@ -222,3 +222,33 @@ out:
return err;
}
+unsigned int vpn_util_resolve_hostname(GResolv *resolv,
+ const char *hostname, GResolvResultFunc func,
+ gpointer user_data)
+{
+ DBG("resolve %s", hostname);
+
+ return g_resolv_lookup_hostname(resolv, hostname, func, user_data);
+}
+
+bool vpn_util_cancel_resolve(GResolv *resolv, unsigned int id)
+{
+ DBG("");
+
+ return g_resolv_cancel_lookup(resolv, id);
+}
+
+GResolv *vpn_util_resolve_new(int index)
+{
+ return g_resolv_new(index);
+}
+
+void vpn_util_resolve_unref(GResolv *resolv)
+{
+ g_resolv_unref(resolv);
+}
+
+int vpn_util_get_resolve_error(GResolv *resolv)
+{
+ return g_resolv_get_error(resolv);
+}
diff --git a/vpn/vpn.h b/vpn/vpn.h
index 1f8c8fcd..88afce45 100644
--- a/vpn/vpn.h
+++ b/vpn/vpn.h
@@ -134,3 +134,13 @@ bool vpn_settings_is_system_user(const char *user);
struct passwd *vpn_util_get_passwd(const char *username);
struct group *vpn_util_get_group(const char *groupname);
int vpn_util_create_path(const char *path, uid_t uid, gid_t grp, int mode);
+
+#include <gweb/gresolv.h>
+
+unsigned int vpn_util_resolve_hostname(GResolv *resolv,
+ const char *hostname, GResolvResultFunc func,
+ gpointer user_data);
+bool vpn_util_cancel_resolve(GResolv *resolv, unsigned int id);
+GResolv *vpn_util_resolve_new(int index);
+void vpn_util_resolve_unref(GResolv *resolv);
+int vpn_util_get_resolve_error(GResolv *resolv);
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 17/45] vpn-util: Add wrappers for GResolv hostname lookup use
2025-07-11 14:27 ` [PATCH 17/45] vpn-util: Add wrappers for GResolv hostname lookup use Jussi Laakkonen
@ 2025-08-04 14:39 ` Denis Kenzior
2025-08-08 12:36 ` Jussi Laakkonen
0 siblings, 1 reply; 69+ messages in thread
From: Denis Kenzior @ 2025-08-04 14:39 UTC (permalink / raw)
To: Jussi Laakkonen, connman
Hi Jussi,
On 7/11/25 9:27 AM, Jussi Laakkonen wrote:
> To avoid linking GResolv with every VPN plugin that uses it, add wrappers
> for the required calls in VPN utils.
This may need a bit more explanation? You're still using GResolv types in these
functions? Why can't g_resolv_ be used instead?
> ---
> vpn/vpn-util.c | 30 ++++++++++++++++++++++++++++++
> vpn/vpn.h | 10 ++++++++++
> 2 files changed, 40 insertions(+)
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 17/45] vpn-util: Add wrappers for GResolv hostname lookup use
2025-08-04 14:39 ` Denis Kenzior
@ 2025-08-08 12:36 ` Jussi Laakkonen
2025-08-08 15:39 ` Denis Kenzior
0 siblings, 1 reply; 69+ messages in thread
From: Jussi Laakkonen @ 2025-08-08 12:36 UTC (permalink / raw)
To: Denis Kenzior, connman
Hi Denis,
On 8/4/25 17:39, Denis Kenzior wrote:
> Hi Jussi,
>
> On 7/11/25 9:27 AM, Jussi Laakkonen wrote:
>> To avoid linking GResolv with every VPN plugin that uses it, add wrappers
>> for the required calls in VPN utils.
>
> This may need a bit more explanation? You're still using GResolv types
> in these functions? Why can't g_resolv_ be used instead?
I can amend the commit message on this. The issue is that VPN plugins
can be either builtin, external (or out of code tree plugins), so making
this wrapper is a bit more straightforward by linking vpnd only with
gresolv and having the plugins not to care about it?
>
>> ---
>> vpn/vpn-util.c | 30 ++++++++++++++++++++++++++++++
>> vpn/vpn.h | 10 ++++++++++
>> 2 files changed, 40 insertions(+)
>>
>
> Regards,
> -Denis
BR,
Jussi
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 17/45] vpn-util: Add wrappers for GResolv hostname lookup use
2025-08-08 12:36 ` Jussi Laakkonen
@ 2025-08-08 15:39 ` Denis Kenzior
0 siblings, 0 replies; 69+ messages in thread
From: Denis Kenzior @ 2025-08-08 15:39 UTC (permalink / raw)
To: Jussi Laakkonen, connman
Hi Jussi,
On 8/8/25 7:36 AM, Jussi Laakkonen wrote:
> Hi Denis,
>
> On 8/4/25 17:39, Denis Kenzior wrote:
>> Hi Jussi,
>>
>> On 7/11/25 9:27 AM, Jussi Laakkonen wrote:
>>> To avoid linking GResolv with every VPN plugin that uses it, add wrappers
>>> for the required calls in VPN utils.
>>
>> This may need a bit more explanation? You're still using GResolv types in
>> these functions? Why can't g_resolv_ be used instead?
>
> I can amend the commit message on this. The issue is that VPN plugins can be
> either builtin, external (or out of code tree plugins), so making this wrapper
> is a bit more straightforward by linking vpnd only with gresolv and having the
> plugins not to care about it?
From a maintenance stand point this is not preferable. Why would we maintain
extra wrappers for a library that is in-tree and we control in the first place?
The plugins should link against GResolv and any issues should be solved at the
build system level.
Regards,
-Denis
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 18/45] wireguard: Use GResolv for DNS reresolve to avoid blocking
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (16 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 17/45] vpn-util: Add wrappers for GResolv hostname lookup use Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 19/45] vpn: Drop state changes from update_provider_state() Jussi Laakkonen
` (29 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Change to use GResolv for the reresolve lookups as with broken
configuration getaddrinfo() will block vpnd for the time it timeouts in
a broken network. By using GResolv the resolving is done first and only
after it is succesful getaddrinfo() is called. This is not done for the
initial connection as the transport medium's network is used, thus it
has no possibility of such misconfiguration as the network is at least
in READY state.
Check the GResolv lookup error and if it is set kill the VPN. This is
because the set error is quite terminal on the connection, better just
to die.
Change reresolve_id to be guint as it should be and handle it properly.
Document why the remove_resolv_cb() and resolv id is used like it is.
---
vpn/plugins/wireguard.c | 162 ++++++++++++++++++++++++++++++++++++----
1 file changed, 147 insertions(+), 15 deletions(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index 2c5981f8..4a3d9ec7 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -43,6 +43,8 @@
#include <connman/setting.h>
#include <connman/vpn-dbus.h>
+#include <gweb/gresolv.h>
+
#include "../vpn-provider.h"
#include "../vpn.h"
@@ -59,7 +61,10 @@ struct wireguard_info {
struct wg_peer peer;
char *endpoint_fqdn;
char *port;
- int reresolve_id;
+ guint reresolve_id;
+ GResolv *resolv;
+ guint resolv_id;
+ guint remove_resolv_id;
};
struct sockaddr_u {
@@ -293,43 +298,120 @@ static bool sockaddr_cmp_addr(struct sockaddr_u *a, struct sockaddr_u *b)
static void run_dns_reresolve(struct wireguard_info *info);
-static gboolean wg_dns_reresolve_cb(gpointer user_data)
+static void remove_resolv(struct wireguard_info *info)
+{
+ DBG("");
+
+ if (info->remove_resolv_id)
+ g_source_remove(info->remove_resolv_id);
+
+ if (info->resolv && info->resolv_id) {
+ DBG("cancel resolv lookup");
+ vpn_util_cancel_resolve(info->resolv, info->resolv_id);
+ }
+
+ info->resolv_id = 0;
+ info->remove_resolv_id = 0;
+
+ vpn_util_resolve_unref(info->resolv);
+ info->resolv = NULL;
+}
+
+static gboolean remove_resolv_cb(gpointer user_data)
+{
+ struct wireguard_info *info = user_data;
+
+ remove_resolv(info);
+
+ return G_SOURCE_REMOVE;
+}
+
+static void resolve_endpoint_cb(GResolvResultStatus status,
+ char **results, gpointer user_data)
{
struct wireguard_info *info = user_data;
struct sockaddr_u addr;
int err;
- if (vpn_provider_get_connection_errors(info->provider) >=
- DNS_RERESOLVE_ERROR_LIMIT) {
- connman_warn("reresolve error limit reached");
- vpn_died(NULL, -ENONET, info->provider);
- return G_SOURCE_REMOVE;
+ DBG("");
+
+ if (!info->resolv && info->resolv_id) {
+ DBG("resolv already removed");
+ return;
}
- /* If this fails after being connected it means configuration error
- * that results in connection errors. */
- err = parse_endpoint(info->endpoint_fqdn,
- info->port, &addr);
+ /*
+ * We cannot unref the resolver here as resolv struct is manipulated
+ * by gresolv.c after we return from this callback. By clearing the
+ * resolv_id no attempt to cancel the lookup that has been executed
+ * here is done.
+ */
+ info->remove_resolv_id = g_timeout_add(0, remove_resolv_cb, info);
+ info->resolv_id = 0;
+
+ switch (status) {
+ case G_RESOLV_RESULT_STATUS_SUCCESS:
+ if (!results || !g_strv_length(results)) {
+ DBG("no resolved results");
+ if (info->provider)
+ vpn_provider_add_error(info->provider,
+ VPN_PROVIDER_ERROR_CONNECT_FAILED);
+
+ return;
+ }
+
+ DBG("resolv success, parse endpoint");
+ break;
+ /* request timeouts or an server issue is not an error, try again */
+ case G_RESOLV_RESULT_STATUS_NO_RESPONSE:
+ case G_RESOLV_RESULT_STATUS_SERVER_FAILURE:
+ DBG("retry DNS reresolve");
+ if (info->provider)
+ vpn_provider_add_error(info->provider,
+ VPN_PROVIDER_ERROR_CONNECT_FAILED);
+
+ run_dns_reresolve(info);
+ return;
+ /* Consider these as non-continuable errors */
+ case G_RESOLV_RESULT_STATUS_ERROR:
+ case G_RESOLV_RESULT_STATUS_FORMAT_ERROR:
+ case G_RESOLV_RESULT_STATUS_NAME_ERROR:
+ case G_RESOLV_RESULT_STATUS_NOT_IMPLEMENTED:
+ case G_RESOLV_RESULT_STATUS_REFUSED:
+ case G_RESOLV_RESULT_STATUS_NO_ANSWER:
+ DBG("stop DNS reresolve");
+ if (info->provider)
+ vpn_provider_add_error(info->provider,
+ VPN_PROVIDER_ERROR_CONNECT_FAILED);
+
+ return;
+ }
+
+ /*
+ * If this fails after being connected it means configuration error
+ * that results in connection errors.
+ */
+ err = parse_endpoint(info->endpoint_fqdn, info->port, &addr);
if (err) {
if (info->provider)
vpn_provider_add_error(info->provider,
VPN_PROVIDER_ERROR_CONNECT_FAILED);
run_dns_reresolve(info);
- return G_SOURCE_REMOVE;
+ return;
}
if (sockaddr_cmp_addr(&addr,
(struct sockaddr_u *)&info->peer.endpoint.addr)) {
run_dns_reresolve(info);
- return G_SOURCE_REMOVE;
+ return;
}
if (addr.sa.sa_family == AF_INET)
memcpy(&info->peer.endpoint.addr, &addr.sin,
- sizeof(info->peer.endpoint.addr4));
+ sizeof(info->peer.endpoint.addr4));
else
memcpy(&info->peer.endpoint.addr, &addr.sin6,
- sizeof(info->peer.endpoint.addr6));
+ sizeof(info->peer.endpoint.addr6));
DBG("Endpoint address has changed, udpate WireGuard device");
err = wg_set_device(&info->device);
@@ -338,6 +420,39 @@ static gboolean wg_dns_reresolve_cb(gpointer user_data)
info->device.name);
run_dns_reresolve(info);
+}
+
+static gboolean wg_dns_reresolve_cb(gpointer user_data)
+{
+ struct wireguard_info *info = user_data;
+ int err;
+
+ DBG("");
+
+ info->reresolve_id = 0;
+
+ if (info->resolv_id > 0) {
+ DBG("previous query was running, abort it");
+ remove_resolv(info);
+ }
+
+ info->resolv = vpn_util_resolve_new(0);
+ if (!info->resolv) {
+ connman_error("cannot create GResolv");
+ return G_SOURCE_REMOVE;
+ }
+
+ info->resolv_id = vpn_util_resolve_hostname(info->resolv,
+ info->endpoint_fqdn,
+ resolve_endpoint_cb, info);
+
+ err = vpn_util_get_resolve_error(info->resolv);
+ if (!info->resolv_id && err) {
+ connman_error("failed to start hostname lookup for %s, err %d",
+ info->endpoint_fqdn, err);
+ vpn_died(NULL, err, info->provider);
+ }
+
return G_SOURCE_REMOVE;
}
@@ -346,6 +461,14 @@ static void run_dns_reresolve(struct wireguard_info *info)
if (info->reresolve_id)
g_source_remove(info->reresolve_id);
+ if (vpn_provider_get_connection_errors(info->provider) >=
+ DNS_RERESOLVE_ERROR_LIMIT) {
+ connman_warn("reresolve error limit reached");
+ vpn_died(NULL, -ENONET, info->provider);
+ info->reresolve_id = 0;
+ return;
+ }
+
info->reresolve_id = g_timeout_add_seconds(DNS_RERESOLVE_TIMEOUT,
wg_dns_reresolve_cb, info);
}
@@ -446,6 +569,12 @@ static int wg_connect(struct vpn_provider *provider,
option = "51820";
gateway = vpn_provider_get_string(provider, "Host");
+ /*
+ * Use the resolve timeout only with re-resolve. Here the network
+ * is setup as the transport is used. In succeeding attempts resolving
+ * is needed as it is done over potentially misconfigured WireGuard
+ * connection that may end up blocking vpnd with getaddrinfo().
+ */
err = parse_endpoint(gateway, option,
(struct sockaddr_u *)&info->peer.endpoint.addr);
if (err) {
@@ -528,6 +657,9 @@ static void wg_disconnect(struct vpn_provider *provider)
if (info->reresolve_id > 0)
g_source_remove(info->reresolve_id);
+ if (info->resolv || info->resolv_id)
+ remove_resolv(info);
+
vpn_provider_set_plugin_data(provider, NULL);
vpn_provider_set_state(provider, VPN_PROVIDER_STATE_DISCONNECT);
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 19/45] vpn: Drop state changes from update_provider_state()
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (17 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 18/45] wireguard: Use GResolv for DNS reresolve to avoid blocking Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 20/45] wireguard: Fix shutdown, ensure one exit and set no agent is used Jussi Laakkonen
` (28 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
It is not necessary to change the state in vpn.c for non-daemon VPNs
because the vpn-provider.c:__vpn_provider_connect() changes the state
according to the state machine. ASSOCIATION state will be assigned for
every VPN after connect, then depending on VPN agent use the state
changes either directly, or after the agent has finished, to CONNECT
state.
---
vpn/plugins/vpn.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/vpn/plugins/vpn.c b/vpn/plugins/vpn.c
index 81a62ff4..2651d4d0 100644
--- a/vpn/plugins/vpn.c
+++ b/vpn/plugins/vpn.c
@@ -556,8 +556,6 @@ static gboolean update_provider_state(gpointer data)
vpn_data->watch = vpn_rtnl_add_newlink_watch(index,
vpn_newlink, provider);
connman_inet_ifup(index);
- vpn_data->state = VPN_STATE_CONNECT;
- vpn_provider_set_state(provider, VPN_PROVIDER_STATE_CONNECT);
return FALSE;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 20/45] wireguard: Fix shutdown, ensure one exit and set no agent is used
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (18 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 19/45] vpn: Drop state changes from update_provider_state() Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 21/45] vpn: Check if disconnect is implemented before calling in stop_vpn() Jussi Laakkonen
` (27 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Split disconnect into wg_disconnect() and disconnect(). Use disconnect()
to properly disconnect when error occurs with the appropriate error
code. Do the same in wg_disconnect() by using the error code to be from
device removal.
The shutdown process requires, that in order to do the proper transition
to IDLE, vpn_died() is to be called. This is normally called by the task
that has been killed so simulate this behavior in WireGuard by adding a
delayed call for that. It is to be executed only once as all the cleanup
is to be done at that last step. Use a delay of 50ms to simulate exit.
Add a function to declare that WireGuard does not use a VPN agent. This
ensures that the state transition is done according to state machine by
__vpn_provider_connect().
Add separate create/free for struct wireguard_info for VPN consistency.
---
vpn/plugins/wireguard.c | 106 ++++++++++++++++++++++++++++++++--------
1 file changed, 86 insertions(+), 20 deletions(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index 4a3d9ec7..ac63b558 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -65,6 +65,7 @@ struct wireguard_info {
GResolv *resolv;
guint resolv_id;
guint remove_resolv_id;
+ guint dying_id;
};
struct sockaddr_u {
@@ -90,6 +91,31 @@ struct {
{"WireGuard.PersistentKeepalive", true}
};
+static struct wireguard_info *create_private_data(struct vpn_provider *provider)
+{
+ struct wireguard_info *info;
+
+ info = g_malloc0(sizeof(struct wireguard_info));
+ info->peer.flags = WGPEER_HAS_PUBLIC_KEY | WGPEER_REPLACE_ALLOWEDIPS;
+ info->device.flags = WGDEVICE_HAS_PRIVATE_KEY;
+ info->device.first_peer = &info->peer;
+ info->device.last_peer = &info->peer;
+ info->provider = vpn_provider_ref(provider);
+
+ return info;
+}
+
+static void free_private_data(struct wireguard_info *info)
+{
+ if (vpn_provider_get_plugin_data(info->provider) == info)
+ vpn_provider_set_plugin_data(info->provider, NULL);
+
+ vpn_provider_unref(info->provider);
+ g_free(info->endpoint_fqdn);
+ g_free(info->port);
+ g_free(info);
+}
+
static int parse_key(const char *str, wg_key key)
{
unsigned char *buf;
@@ -422,6 +448,8 @@ static void resolve_endpoint_cb(GResolvResultStatus status,
run_dns_reresolve(info);
}
+static int disconnect(struct vpn_provider *provider, int error);
+
static gboolean wg_dns_reresolve_cb(gpointer user_data)
{
struct wireguard_info *info = user_data;
@@ -450,7 +478,7 @@ static gboolean wg_dns_reresolve_cb(gpointer user_data)
if (!info->resolv_id && err) {
connman_error("failed to start hostname lookup for %s, err %d",
info->endpoint_fqdn, err);
- vpn_died(NULL, err, info->provider);
+ disconnect(info->provider, err);
}
return G_SOURCE_REMOVE;
@@ -464,7 +492,7 @@ static void run_dns_reresolve(struct wireguard_info *info)
if (vpn_provider_get_connection_errors(info->provider) >=
DNS_RERESOLVE_ERROR_LIMIT) {
connman_warn("reresolve error limit reached");
- vpn_died(NULL, -ENONET, info->provider);
+ disconnect(info->provider, -ENONET);
info->reresolve_id = 0;
return;
}
@@ -484,12 +512,7 @@ static int wg_connect(struct vpn_provider *provider,
char *ifname;
int err = -EINVAL;
- info = g_malloc0(sizeof(struct wireguard_info));
- info->peer.flags = WGPEER_HAS_PUBLIC_KEY | WGPEER_REPLACE_ALLOWEDIPS;
- info->device.flags = WGDEVICE_HAS_PRIVATE_KEY;
- info->device.first_peer = &info->peer;
- info->device.last_peer = &info->peer;
- info->provider = vpn_provider_ref(provider);
+ info = create_private_data(provider);
DBG("");
@@ -643,8 +666,34 @@ error:
goto done;
}
-static void wg_disconnect(struct vpn_provider *provider)
+struct wireguard_exit_data {
+ struct vpn_provider *provider;
+ int err;
+};
+
+static gboolean wg_died(gpointer user_data)
{
+ struct wireguard_exit_data *data = user_data;
+ struct wireguard_info *info;
+
+ DBG("");
+
+ /* No task for no daemon VPN - use vpn_died() with no task. */
+ vpn_died(NULL, data->err, data->provider);
+
+ info = vpn_provider_get_plugin_data(data->provider);
+ if (info)
+ free_private_data(info);
+
+ g_free(data);
+
+ return G_SOURCE_REMOVE;
+}
+
+/* Allow to overrule the exit code for vpn_died */
+static int disconnect(struct vpn_provider *provider, int err)
+{
+ struct wireguard_exit_data *data;
struct wireguard_info *info;
int exit_code;
@@ -652,7 +701,10 @@ static void wg_disconnect(struct vpn_provider *provider)
info = vpn_provider_get_plugin_data(provider);
if (!info)
- return;
+ return -ENODATA;
+
+ if (info->dying_id)
+ return -EALREADY;
if (info->reresolve_id > 0)
g_source_remove(info->reresolve_id);
@@ -660,21 +712,29 @@ static void wg_disconnect(struct vpn_provider *provider)
if (info->resolv || info->resolv_id)
remove_resolv(info);
- vpn_provider_set_plugin_data(provider, NULL);
-
vpn_provider_set_state(provider, VPN_PROVIDER_STATE_DISCONNECT);
exit_code = wg_del_device(info->device.name);
- vpn_provider_unref(info->provider);
- g_free(info->endpoint_fqdn);
- g_free(info->port);
- g_free(info);
+ /* Simulate a task-running VPN to issue vpn_died after exiting this */
+ data = g_malloc0(sizeof(struct wireguard_exit_data));
+ data->provider = provider;
+ data->err = err ? err : exit_code;
- DBG("exiting with %d", exit_code);
+ info->dying_id = g_timeout_add(50, wg_died, data);
- /* No task for no daemon VPN - use VPN died with no task. */
- vpn_died(NULL, exit_code, provider);
+ return exit_code;
+}
+
+static void wg_disconnect(struct vpn_provider *provider)
+{
+ int exit_code;
+
+ DBG("");
+
+ exit_code = disconnect(provider, 0);
+
+ DBG("exited with %d", exit_code);
}
static int wg_error_code(struct vpn_provider *provider, int exit_code)
@@ -711,12 +771,18 @@ static int wg_save(struct vpn_provider *provider, GKeyFile *keyfile)
return 0;
}
+bool wg_uses_vpn_agent(struct vpn_provider *provider)
+{
+ return false;
+}
+
static struct vpn_driver vpn_driver = {
.flags = VPN_FLAG_NO_TUN | VPN_FLAG_NO_DAEMON,
.connect = wg_connect,
.disconnect = wg_disconnect,
.save = wg_save,
- .error_code = wg_error_code
+ .error_code = wg_error_code,
+ .uses_vpn_agent = wg_uses_vpn_agent
};
static int wg_init(void)
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 21/45] vpn: Check if disconnect is implemented before calling in stop_vpn()
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (19 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 20/45] wireguard: Fix shutdown, ensure one exit and set no agent is used Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 22/45] wireguard: Tokenize host for getaddrinfo() Jussi Laakkonen
` (26 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Check if the disconnect() is implemented for the vpn_driver before
attempting to call it when stopping a VPN. Also cleanup the code a bit.
Amends changes made in ffec305c1c93301534144774fffebb25fb9c6c7a
---
vpn/plugins/vpn.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/vpn/plugins/vpn.c b/vpn/plugins/vpn.c
index 2651d4d0..a6b738b0 100644
--- a/vpn/plugins/vpn.c
+++ b/vpn/plugins/vpn.c
@@ -76,6 +76,7 @@ static int stop_vpn(struct vpn_provider *provider)
{
struct vpn_data *data = vpn_provider_get_data(provider);
struct vpn_driver_data *vpn_driver_data;
+ const struct vpn_driver *vpn_driver = NULL;
const char *name;
struct ifreq ifr;
int fd, err;
@@ -88,16 +89,19 @@ static int stop_vpn(struct vpn_provider *provider)
return -EINVAL;
vpn_driver_data = g_hash_table_lookup(driver_hash, name);
+ if (vpn_driver_data)
+ vpn_driver = vpn_driver_data->vpn_driver;
- if (vpn_driver_data && vpn_driver_data->vpn_driver &&
- vpn_driver_data->vpn_driver->flags & VPN_FLAG_NO_TUN) {
+ if (vpn_driver && vpn_driver->flags & VPN_FLAG_NO_TUN) {
/*
* Disconnect only VPNs with daemon, otherwise in error return
* there is a double free with vpn_died() or the failure state
* is overridden by changes made by disconnect to state.
*/
- if (!(vpn_driver_data->vpn_driver->flags & VPN_FLAG_NO_DAEMON))
- vpn_driver_data->vpn_driver->disconnect(data->provider);
+ if (!(vpn_driver->flags & VPN_FLAG_NO_DAEMON) &&
+ vpn_driver->disconnect)
+ vpn_driver->disconnect(data->provider);
+
return 0;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 22/45] wireguard: Tokenize host for getaddrinfo()
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (20 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 21/45] vpn: Check if disconnect is implemented before calling in stop_vpn() Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 23/45] util: Add address family set/get/reset helpers Jussi Laakkonen
` (25 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
The host parameter contains the address and netmask in CIDR notation
with IPv6 to define also the PrefixLength to be used in D-Bus for
connmand. getaddrinfo() relies inet_pton() that does not work with the
notation so tokenize host before passing it to getaddrinfo().
---
vpn/plugins/wireguard.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index ac63b558..e2714c54 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -187,8 +187,25 @@ static int parse_endpoint(const char *host, const char *port, struct sockaddr_u
{
struct addrinfo hints;
struct addrinfo *result, *rp;
+ char **tokens;
int sk;
int err;
+ unsigned int len;
+
+ /*
+ * getaddrinfo() relies on inet_pton() that suggests using addresses
+ * without CIDR notation. Host should contain the address in CIDR
+ * notation to be able to pass the prefix length to ConnMan via D-Bus.
+ */
+ tokens = g_strsplit(host, "/", -1);
+ len = g_strv_length(tokens);
+ if (len > 2 || len < 1) {
+ DBG("Failure tokenizing host %s", host);
+ g_strfreev(tokens);
+ return -EINVAL;
+ }
+
+ DBG("using host %s", tokens[0]);
memset(&hints, 0, sizeof(struct addrinfo));
hints.ai_family = AF_UNSPEC;
@@ -196,7 +213,9 @@ static int parse_endpoint(const char *host, const char *port, struct sockaddr_u
hints.ai_flags = 0;
hints.ai_protocol = 0;
- err = getaddrinfo(host, port, &hints, &result);
+ err = getaddrinfo(tokens[0], port, &hints, &result);
+ g_strfreev(tokens);
+
if (err < 0) {
DBG("Failed to resolve host address: %s", gai_strerror(err));
return -EINVAL;
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 23/45] util: Add address family set/get/reset helpers
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (21 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 22/45] wireguard: Tokenize host for getaddrinfo() Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-08-04 14:47 ` Denis Kenzior
2025-07-11 14:27 ` [PATCH 24/45] vpn-provider: Add support for dual-IP VPNs Jussi Laakkonen
` (24 subsequent siblings)
47 siblings, 1 reply; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
[util] Add address family set/get/reset helpers. JB#62713
Add address family (af) helper functions to be used with a boolean array
defining which address families a provider has. Supports AF_INET and
AF_INET6 as set/get parameters. The array must be of AF_ARRAY_LENGTH
size.
---
src/shared/util.c | 40 ++++++++++++++++++++++++++++++++++++++++
src/shared/util.h | 11 +++++++++++
2 files changed, 51 insertions(+)
diff --git a/src/shared/util.c b/src/shared/util.c
index bda2d2b3..22114029 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -128,3 +128,43 @@ char *util_timeval_to_iso8601(struct timeval *time)
return g_strdup(buf);
}
+
+void util_set_afs(bool *afs, int family)
+{
+ if (!afs)
+ return;
+
+ switch (family) {
+ case AF_INET:
+ afs[AF_INET_POS] = true;
+ break;
+ case AF_INET6:
+ afs[AF_INET6_POS] = true;
+ break;
+ default:
+ break;
+ }
+}
+
+bool util_get_afs(bool *afs, int family)
+{
+ if (!afs)
+ return false;
+
+ switch (family) {
+ case AF_INET:
+ return afs[AF_INET_POS];
+ case AF_INET6:
+ return afs[AF_INET6_POS];
+ default:
+ return false;
+ }
+}
+
+void util_reset_afs(bool *afs)
+{
+ if (!afs)
+ return;
+
+ afs[AF_INET_POS] = afs[AF_INET6_POS] = false;
+}
diff --git a/src/shared/util.h b/src/shared/util.h
index 430b821f..c5a763c4 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -24,6 +24,13 @@
#include <sys/time.h>
#include <glib.h>
+#include <stdbool.h>
+#include <inet.h>
+#include <arpa/inet.h>
+
+#define AF_INET_POS 0
+#define AF_INET6_POS 1
+#define AF_ARRAY_LENGTH 2
typedef void (*util_debug_func_t)(const char *str, void *user_data);
@@ -53,3 +60,7 @@ static inline struct cb_data *cb_data_new(void *cb, void *user_data)
void util_iso8601_to_timeval(char *str, struct timeval *time);
char *util_timeval_to_iso8601(struct timeval *time);
+
+void util_set_afs(bool *afs, int family);
+bool util_get_afs(bool *afs, int family);
+void util_reset_afs(bool *afs);
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 23/45] util: Add address family set/get/reset helpers
2025-07-11 14:27 ` [PATCH 23/45] util: Add address family set/get/reset helpers Jussi Laakkonen
@ 2025-08-04 14:47 ` Denis Kenzior
2025-08-08 12:29 ` Jussi Laakkonen
0 siblings, 1 reply; 69+ messages in thread
From: Denis Kenzior @ 2025-08-04 14:47 UTC (permalink / raw)
To: Jussi Laakkonen, connman
Hi Jussi,
On 7/11/25 9:27 AM, Jussi Laakkonen wrote:
> [util] Add address family set/get/reset helpers. JB#62713
I don't know what this is. Please drop any references to internal tickets when
submitting and spell out what the commit is trying to do in the commit description.
>
> Add address family (af) helper functions to be used with a boolean array
> defining which address families a provider has. Supports AF_INET and
> AF_INET6 as set/get parameters. The array must be of AF_ARRAY_LENGTH
> size.
Ugh, seriously? Maybe:
bool support_ipv6 : 1;
bool support_ipv4 : 1;
?
> ---
> src/shared/util.c | 40 ++++++++++++++++++++++++++++++++++++++++
> src/shared/util.h | 11 +++++++++++
> 2 files changed, 51 insertions(+)
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 23/45] util: Add address family set/get/reset helpers
2025-08-04 14:47 ` Denis Kenzior
@ 2025-08-08 12:29 ` Jussi Laakkonen
0 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-08-08 12:29 UTC (permalink / raw)
To: Denis Kenzior, connman
Hi Denis,
On 8/4/25 17:47, Denis Kenzior wrote:
> Hi Jussi,
>
> On 7/11/25 9:27 AM, Jussi Laakkonen wrote:
>> [util] Add address family set/get/reset helpers. JB#62713
>
> I don't know what this is. Please drop any references to internal
> tickets when submitting and spell out what the commit is trying to do in
> the commit description.
Oops I apparently missed one here. We have the internal tickets on every
commit so 1/46 was a simple oversight. I'll remove it and put v2 next week.
>
>>
>> Add address family (af) helper functions to be used with a boolean array
>> defining which address families a provider has. Supports AF_INET and
>> AF_INET6 as set/get parameters. The array must be of AF_ARRAY_LENGTH
>> size.
>
> Ugh, seriously? Maybe:
> bool support_ipv6 : 1;
> bool support_ipv4 : 1;
>
> ?
Yeah it can be made into a struct for sure. I'll make V2 of related
changes (hopefully) next week.
>
>> ---
>> src/shared/util.c | 40 ++++++++++++++++++++++++++++++++++++++++
>> src/shared/util.h | 11 +++++++++++
>> 2 files changed, 51 insertions(+)
>>
>
> Regards,
> -Denis
BR,
Jussi
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 24/45] vpn-provider: Add support for dual-IP VPNs
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (22 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 23/45] util: Add address family set/get/reset helpers Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 25/45] provider: " Jussi Laakkonen
` (23 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
WireGuard can use both IP addresses, for example. Use a boolean array to
define AFs for a VPN.
---
Makefile.am | 3 ++-
vpn/vpn-provider.c | 64 ++++++++++++++++++++++++++++++----------------
2 files changed, 44 insertions(+), 23 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 3dc3bb5c..28a8b7d5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -181,7 +181,8 @@ vpn_connman_vpnd_SOURCES = $(builtin_vpn_sources) $(backtrace_sources) \
vpn/vpn-ipconfig.c src/inet.c vpn/vpn-rtnl.c \
src/dbus.c src/storage.c src/ipaddress.c src/agent.c \
vpn/vpn-agent.c vpn/vpn-agent.h src/inotify.c \
- vpn/vpn-config.c vpn/vpn-settings.c vpn/vpn-util.c
+ vpn/vpn-config.c vpn/vpn-settings.c vpn/vpn-util.c \
+ src/shared/util.c
vpn_connman_vpnd_LDADD = gdbus/libgdbus-internal.la $(builtin_vpn_libadd) \
@GIO_LIBS@ @GLIB_LIBS@ @DBUS_LIBS@ @GNUTLS_LIBS@ \
diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index b21e9e61..7aa72ef0 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -55,6 +55,7 @@
#include "vpn-provider.h"
#include "vpn.h"
#include "plugins/vpn.h"
+#include "../src/shared/util.h"
static DBusConnection *connection;
static GHashTable *provider_hash;
@@ -85,7 +86,7 @@ struct vpn_provider {
char *type;
char *host;
char *domain;
- int family;
+ bool family[AF_ARRAY_LENGTH];
bool do_split_routing;
GHashTable *routes;
struct vpn_provider_driver *driver;
@@ -127,6 +128,31 @@ static guint connman_service_watch;
static bool connman_online;
static bool state_query_completed;
+
+static bool provider_get_family(struct vpn_provider *provider, int family)
+{
+ if (!provider)
+ return false;
+
+ return util_get_afs(provider->family, family);
+}
+
+static void provider_set_family(struct vpn_provider *provider, int family)
+{
+ if (!provider)
+ return;
+
+ util_set_afs(provider->family, family);
+}
+
+static void provider_reset_family(struct vpn_provider *provider)
+{
+ if (!provider)
+ return;
+
+ util_reset_afs(provider->family);
+}
+
static void append_properties(DBusMessageIter *iter,
struct vpn_provider *provider);
static int vpn_provider_save(struct vpn_provider *provider);
@@ -1875,11 +1901,11 @@ static int provider_indicate_state(struct vpn_provider *provider,
VPN_CONNECTION_INTERFACE, "Index",
DBUS_TYPE_INT32, &provider->index);
- if (provider->family == AF_INET)
+ if (provider_get_family(provider, AF_INET))
connman_dbus_property_changed_dict(provider->path,
VPN_CONNECTION_INTERFACE, "IPv4",
append_ipv4, provider);
- else if (provider->family == AF_INET6)
+ if (provider_get_family(provider, AF_INET6))
connman_dbus_property_changed_dict(provider->path,
VPN_CONNECTION_INTERFACE, "IPv6",
append_ipv6, provider);
@@ -1976,10 +2002,10 @@ static void append_properties(DBusMessageIter *iter,
connman_dbus_dict_append_basic(&dict, "SplitRouting",
DBUS_TYPE_BOOLEAN, &split_routing);
- if (provider->family == AF_INET)
+ if (provider_get_family(provider, AF_INET))
connman_dbus_dict_append_dict(&dict, "IPv4", append_ipv4,
provider);
- else if (provider->family == AF_INET6)
+ if (provider_get_family(provider, AF_INET6))
connman_dbus_dict_append_dict(&dict, "IPv6", append_ipv6,
provider);
@@ -2032,18 +2058,16 @@ static void connection_added_signal(struct vpn_provider *provider)
static int set_connected(struct vpn_provider *provider,
bool connected)
{
- struct vpn_ipconfig *ipconfig;
-
DBG("provider %p id %s connected %d", provider,
provider->identifier, connected);
if (connected) {
- if (provider->family == AF_INET6)
- ipconfig = provider->ipconfig_ipv6;
- else
- ipconfig = provider->ipconfig_ipv4;
-
- __vpn_ipconfig_address_add(ipconfig, provider->family);
+ if (provider_get_family(provider, AF_INET))
+ __vpn_ipconfig_address_add(provider->ipconfig_ipv4,
+ AF_INET);
+ if (provider_get_family(provider, AF_INET6))
+ __vpn_ipconfig_address_add(provider->ipconfig_ipv6,
+ AF_INET6);
provider_indicate_state(provider,
VPN_PROVIDER_STATE_READY);
@@ -2053,6 +2077,7 @@ static int set_connected(struct vpn_provider *provider,
provider_indicate_state(provider,
VPN_PROVIDER_STATE_IDLE);
+ provider_reset_family(provider);
}
return 0;
@@ -3077,7 +3102,7 @@ int vpn_provider_set_ipaddress(struct vpn_provider *provider,
if (!ipconfig)
return -EINVAL;
- provider->family = ipaddress->family;
+ provider_set_family(provider, ipaddress->family);
if (provider->state == VPN_PROVIDER_STATE_CONNECT ||
provider->state == VPN_PROVIDER_STATE_READY) {
@@ -3367,18 +3392,13 @@ unsigned int vpn_provider_get_connection_errors(
void vpn_provider_change_address(struct vpn_provider *provider)
{
- switch (provider->family) {
- case AF_INET:
+ if (provider_get_family(provider, AF_INET))
connman_inet_set_address(provider->index,
__vpn_ipconfig_get_address(provider->ipconfig_ipv4));
- break;
- case AF_INET6:
+
+ if (provider_get_family(provider, AF_INET6))
connman_inet_set_ipv6_address(provider->index,
__vpn_ipconfig_get_address(provider->ipconfig_ipv6));
- break;
- default:
- break;
- }
}
void vpn_provider_clear_address(struct vpn_provider *provider, int family)
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 25/45] provider: Add support for dual-IP VPNs
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (23 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 24/45] vpn-provider: Add support for dual-IP VPNs Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 26/45] vpn: " Jussi Laakkonen
` (22 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Handle both IP family types for a VPN if it, like WireGuard, has them.
Use a boolean array to define AFs for a VPN.
Change the functionality regarding supported families to use a boolean
array defining which address families are supported by a VPN.
Modify the connman_provider_get_family() to be a query type based on the
AF given.
---
include/provider.h | 2 +-
src/provider.c | 88 +++++++++++++++++++++++++++++++++-------------
2 files changed, 65 insertions(+), 25 deletions(-)
diff --git a/include/provider.h b/include/provider.h
index aac47527..3e6ea2c7 100644
--- a/include/provider.h
+++ b/include/provider.h
@@ -117,7 +117,7 @@ void connman_provider_set_autoconnect(struct connman_provider *provider,
bool connman_provider_is_split_routing(struct connman_provider *provider);
int connman_provider_set_split_routing(struct connman_provider *provider,
bool split_routing);
-int connman_provider_get_family(struct connman_provider *provider);
+bool connman_provider_get_family(struct connman_provider *provider, int family);
const char *connman_provider_get_driver_name(struct connman_provider *provider);
const char *connman_provider_get_save_group(struct connman_provider *provider);
diff --git a/src/provider.c b/src/provider.c
index ab4aeafb..e790ccea 100644
--- a/src/provider.c
+++ b/src/provider.c
@@ -31,6 +31,7 @@
#include <gweb/gresolv.h>
#include "connman.h"
+#include "src/shared/util.h"
static DBusConnection *connection = NULL;
@@ -44,11 +45,35 @@ struct connman_provider {
struct connman_service *vpn_service;
int index;
char *identifier;
- int family;
+ bool family[AF_ARRAY_LENGTH];
struct connman_provider_driver *driver;
void *driver_data;
};
+static void provider_set_family(struct connman_provider *provider, int family)
+{
+ if (!provider)
+ return;
+
+ util_set_afs(provider->family, family);
+}
+
+static bool provider_get_family(struct connman_provider *provider, int family)
+{
+ if (!provider)
+ return false;
+
+ return util_get_afs(provider->family, family);
+}
+
+static void provider_reset_family(struct connman_provider *provider)
+{
+ if (!provider)
+ return;
+
+ util_reset_afs(provider->family);
+}
+
void __connman_provider_append_properties(struct connman_provider *provider,
DBusMessageIter *iter)
{
@@ -171,6 +196,8 @@ int connman_provider_disconnect(struct connman_provider *provider)
provider_indicate_state(provider,
CONNMAN_SERVICE_STATE_IDLE);
+ provider_reset_family(provider);
+
return 0;
}
@@ -258,22 +285,36 @@ static int set_connected(struct connman_provider *provider,
bool connected)
{
struct connman_service *service = provider->vpn_service;
- struct connman_ipconfig *ipconfig;
+ struct connman_ipconfig *ipconfig_ipv4 = NULL;
+ struct connman_ipconfig *ipconfig_ipv6 = NULL;
if (!service)
return -ENODEV;
- ipconfig = __connman_service_get_ipconfig(service, provider->family);
+ if (provider_get_family(provider, AF_INET))
+ ipconfig_ipv4 = __connman_service_get_ipconfig(service,
+ AF_INET);
+
+ if (provider_get_family(provider, AF_INET6))
+ ipconfig_ipv6 = __connman_service_get_ipconfig(service,
+ AF_INET6);
if (connected) {
- if (!ipconfig) {
+ if (!ipconfig_ipv4 && !ipconfig_ipv6) {
provider_indicate_state(provider,
CONNMAN_SERVICE_STATE_FAILURE);
return -EIO;
}
- __connman_ipconfig_address_add(ipconfig);
- __connman_ipconfig_gateway_add(ipconfig);
+ if (ipconfig_ipv4) {
+ __connman_ipconfig_address_add(ipconfig_ipv4);
+ __connman_ipconfig_gateway_add(ipconfig_ipv4);
+ }
+
+ if (ipconfig_ipv6) {
+ __connman_ipconfig_address_add(ipconfig_ipv6);
+ __connman_ipconfig_gateway_add(ipconfig_ipv6);
+ }
provider_indicate_state(provider,
CONNMAN_SERVICE_STATE_READY);
@@ -283,10 +324,16 @@ static int set_connected(struct connman_provider *provider,
CONNMAN_PROVIDER_ROUTE_ALL);
} else {
- if (ipconfig) {
+ if (ipconfig_ipv4) {
provider_indicate_state(provider,
CONNMAN_SERVICE_STATE_DISCONNECT);
- __connman_ipconfig_gateway_remove(ipconfig);
+ __connman_ipconfig_gateway_remove(ipconfig_ipv4);
+ }
+
+ if (ipconfig_ipv6) {
+ provider_indicate_state(provider,
+ CONNMAN_SERVICE_STATE_DISCONNECT);
+ __connman_ipconfig_gateway_remove(ipconfig_ipv6);
}
provider_indicate_state(provider,
@@ -570,7 +617,7 @@ int connman_provider_set_ipaddress(struct connman_provider *provider,
if (!ipconfig)
return -EINVAL;
- provider->family = ipaddress->family;
+ provider_set_family(provider, ipaddress->family);
__connman_ipconfig_set_method(ipconfig, CONNMAN_IPCONFIG_METHOD_FIXED);
@@ -665,19 +712,15 @@ int connman_provider_set_split_routing(struct connman_provider *provider,
return -EALREADY;
}
- switch (provider->family) {
- case AF_INET:
+ if (provider_get_family(provider, AF_INET) &&
+ provider_get_family(provider, AF_INET6))
+ type = CONNMAN_IPCONFIG_TYPE_ALL;
+ else if (provider_get_family(provider, AF_INET))
type = CONNMAN_IPCONFIG_TYPE_IPV4;
- break;
- case AF_INET6:
+ else if (provider_get_family(provider, AF_INET6))
type = CONNMAN_IPCONFIG_TYPE_IPV6;
- break;
- case AF_UNSPEC:
- type = CONNMAN_IPCONFIG_TYPE_ALL;
- break;
- default:
+ else
type = CONNMAN_IPCONFIG_TYPE_UNKNOWN;
- }
if (!__connman_service_is_connected_state(provider->vpn_service,
type)) {
@@ -719,12 +762,9 @@ out:
return err;
}
-int connman_provider_get_family(struct connman_provider *provider)
+bool connman_provider_get_family(struct connman_provider *provider, int family)
{
- if (!provider)
- return AF_UNSPEC;
-
- return provider->family;
+ return provider_get_family(provider, family);
}
static void unregister_provider(gpointer data)
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 26/45] vpn: Add support for dual-IP VPNs
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (24 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 25/45] provider: " Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 27/45] wireguard: Support both IPv4 and IPv6 address Jussi Laakkonen
` (21 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Information sent by vpn-provider.c on dual-IP VPNs must be handled also
here properly. Adapt to using modified connman_provider_get_family().
Use with the modified connman_provider_get_family() to query which
families are supported to clean up the code.
---
plugins/vpn.c | 126 +++++++++++++++++++++++++++++++++-----------------
1 file changed, 84 insertions(+), 42 deletions(-)
diff --git a/plugins/vpn.c b/plugins/vpn.c
index d683c67d..46b3a0fb 100644
--- a/plugins/vpn.c
+++ b/plugins/vpn.c
@@ -1554,57 +1554,94 @@ static void set_route(struct connection_data *data, struct vpn_route *route)
static int save_route(GHashTable *routes, int family, const char *network,
const char *netmask, const char *gateway);
-static int add_network_route(struct connection_data *data)
+
+static int set_network_route(struct connection_data *data, struct vpn_route *rt)
{
- struct vpn_route rt = { 0, };
int err;
- if (!data)
- return -EINVAL;
-
- rt.family = connman_provider_get_family(data->provider);
- switch (rt.family) {
- case PF_INET:
- err = connman_inet_get_route_addresses(data->index,
- &rt.network, &rt.netmask, &rt.gateway);
- break;
- case PF_INET6:
- err = connman_inet_ipv6_get_route_addresses(data->index,
- &rt.network, &rt.netmask, &rt.gateway);
- break;
- default:
- connman_error("invalid protocol family %d", rt.family);
+ if (!data || !rt)
return -EINVAL;
- }
DBG("network %s gateway %s netmask %s for provider %p",
- rt.network, rt.gateway, rt.netmask,
- data->provider);
+ rt->network, rt->gateway, rt->netmask,
+ data->provider);
- if (err) {
- connman_error("cannot get network/gateway/netmask for %p",
- data->provider);
- goto out;
- }
-
- err = save_route(data->server_routes, rt.family, rt.network, rt.netmask,
- rt.gateway);
+ err = save_route(data->server_routes, rt->family, rt->network,
+ rt->netmask, rt->gateway);
if (err) {
connman_warn("failed to add network route for provider"
"%p", data->provider);
- goto out;
+ return err;
}
- set_route(data, &rt);
-
-out:
- g_free(rt.network);
- g_free(rt.netmask);
- g_free(rt.gateway);
+ set_route(data, rt);
return 0;
}
+static void free_network_route(struct vpn_route *rt)
+{
+ g_free(rt->network);
+ rt->network = NULL;
+
+ g_free(rt->netmask);
+ rt->network = NULL;
+
+ g_free(rt->gateway);
+ rt->network = NULL;
+}
+
+static int add_network_route(struct connection_data *data)
+{
+ struct vpn_route rt = { 0, };
+ int err = 0;
+
+ if (!data)
+ return -EINVAL;
+
+ if (connman_provider_get_family(data->provider, AF_INET)) {
+ rt.family = AF_INET;
+ err = connman_inet_get_route_addresses(data->index,
+ &rt.network, &rt.netmask,
+ &rt.gateway);
+ if (err) {
+ connman_error("cannot get IPv4 network/gateway/netmask "
+ "for %p/%s", data->provider,
+ data->ident);
+ } else {
+ err = set_network_route(data, &rt);
+ if (err)
+ connman_error("cannot set IPv4 network route "
+ "for %p/%s", data->provider,
+ data->ident);
+ }
+
+ free_network_route(&rt);
+ }
+
+ if (connman_provider_get_family(data->provider, AF_INET6)) {
+ rt.family = AF_INET6;
+ err = connman_inet_ipv6_get_route_addresses(data->index,
+ &rt.network, &rt.netmask,
+ &rt.gateway);
+ if (err) {
+ connman_error("cannot get IPv6 network/gateway/netmask "
+ "for %p/%s", data->provider,
+ data->ident);
+ } else {
+ err = set_network_route(data, &rt);
+ if (err)
+ connman_error("cannot set IPv6 network route "
+ "for %p/%s", data->provider,
+ data->ident);
+ }
+
+ free_network_route(&rt);
+ }
+
+ return err;
+}
+
static bool is_valid_route_table(struct connman_provider *provider,
GHashTable *table)
{
@@ -1667,6 +1704,14 @@ static bool check_routes(struct connman_provider *provider)
return false;
}
+static void set_default_route(struct connection_data *data, int family,
+ char *ipaddr_any)
+{
+ struct vpn_route def_route = {family, ipaddr_any, ipaddr_any, NULL};
+
+ set_route(data, &def_route);
+}
+
static int set_routes(struct connman_provider *provider,
enum connman_provider_route_type type)
{
@@ -1699,13 +1744,10 @@ static int set_routes(struct connman_provider *provider,
/* If non-split routed VPN does not have a default route, add it */
if (!connman_provider_is_split_routing(provider) &&
!data->default_route_set) {
- int family = connman_provider_get_family(provider);
- const char *ipaddr_any = family == AF_INET6 ?
- "::" : "0.0.0.0";
- struct vpn_route def_route = {family, (char*) ipaddr_any,
- (char*) ipaddr_any, NULL};
-
- set_route(data, &def_route);
+ if (connman_provider_get_family(provider, AF_INET))
+ set_default_route(data, AF_INET, "0.0.0.0");
+ if (connman_provider_get_family(provider, AF_INET6))
+ set_default_route(data, AF_INET6, "::");
}
/* Split routed VPN must have at least one route to the network */
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 27/45] wireguard: Support both IPv4 and IPv6 address
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (25 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 26/45] vpn: " Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 28/45] inet: Expose __connman_inet_is_any_addr() for plugins to use Jussi Laakkonen
` (20 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
In the configurations there is a possibility to add multiple entries,
separated by commas, for "Address". Implement supporting this in the
plugin via the plugin's WireGuard.Address option.
Only the first valid IPv4 and IPv6 addresses are utilized. The rest are
simply ignored.
---
vpn/plugins/wireguard.c | 116 +++++++++++++++++++++++++++++-----------
1 file changed, 85 insertions(+), 31 deletions(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index e2714c54..2783d9bd 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -245,50 +245,97 @@ static int parse_endpoint(const char *host, const char *port, struct sockaddr_u
return 0;
}
-static int parse_address(const char *address, const char *gateway,
- struct connman_ipaddress **ipaddress)
+struct wg_ipaddresses {
+ struct connman_ipaddress *ipaddress_ipv4;
+ struct connman_ipaddress *ipaddress_ipv6;
+};
+
+static int parse_addresses(const char *address, const char *gateway,
+ struct wg_ipaddresses *ipaddresses)
{
char buf[INET6_ADDRSTRLEN];
unsigned char prefixlen;
+ char **addresses;
char **tokens;
char *end, *netmask;
int err;
+ int i;
- tokens = g_strsplit(address, "/", -1);
- if (g_strv_length(tokens) != 2) {
- g_strfreev(tokens);
+ addresses = g_strsplit(address, ", ", -1);
+ if (!g_strv_length(addresses)) {
+ g_strfreev(addresses);
return -EINVAL;
}
- prefixlen = g_ascii_strtoull(tokens[1], &end, 10);
+ for (i = 0; addresses[i]; i++) {
+ struct connman_ipaddress *ipaddress = NULL;
+ int family = 0;
+
+ tokens = g_strsplit(addresses[i], "/", -1);
+ if (g_strv_length(tokens) != 2) {
+ g_strfreev(tokens);
+ DBG("Invalid Wireguard.Address value %s", addresses[i]);
+ continue;
+ }
- if (inet_pton(AF_INET, tokens[0], buf) == 1) {
- netmask = g_strdup_printf("%d.%d.%d.%d",
+ DBG("address %s", addresses[i]);
+
+ prefixlen = g_ascii_strtoull(tokens[1], &end, 10);
+
+ if (inet_pton(AF_INET, tokens[0], buf) == 1) {
+ if (ipaddresses->ipaddress_ipv4) {
+ g_strfreev(tokens);
+ DBG("IPv4 address already set");
+ err = -EALREADY;
+ continue;
+ }
+
+ family = AF_INET;
+ netmask = g_strdup_printf("%d.%d.%d.%d",
((0xffffffff << (32 - prefixlen)) >> 24) & 0xff,
((0xffffffff << (32 - prefixlen)) >> 16) & 0xff,
((0xffffffff << (32 - prefixlen)) >> 8) & 0xff,
((0xffffffff << (32 - prefixlen)) >> 0) & 0xff);
- *ipaddress = connman_ipaddress_alloc(AF_INET);
- err = connman_ipaddress_set_ipv4(*ipaddress, tokens[0],
- netmask, gateway);
- g_free(netmask);
- } else if (inet_pton(AF_INET6, tokens[0], buf) == 1) {
- *ipaddress = connman_ipaddress_alloc(AF_INET6);
- err = connman_ipaddress_set_ipv6(*ipaddress, tokens[0],
- prefixlen, gateway);
- } else {
- DBG("Invalid Wireguard.Address value");
- err = -EINVAL;
- }
+ ipaddress = connman_ipaddress_alloc(family);
+ err = connman_ipaddress_set_ipv4(ipaddress, tokens[0],
+ netmask, gateway);
+ g_free(netmask);
+ } else if (inet_pton(AF_INET6, tokens[0], buf) == 1) {
+ if (ipaddresses->ipaddress_ipv6) {
+ g_strfreev(tokens);
+ DBG("IPv6 address already set");
+ err = -EALREADY;
+ continue;
+ }
+
+ family = AF_INET6;
+ ipaddress = connman_ipaddress_alloc(family);
+ err = connman_ipaddress_set_ipv6(ipaddress, tokens[0],
+ prefixlen, gateway);
+ } else {
+ DBG("Invalid Wireguard.Address value");
+ err = -EINVAL;
+ }
- connman_ipaddress_set_p2p(*ipaddress, true);
+ g_strfreev(tokens);
+ if (err) {
+ connman_ipaddress_free(ipaddress);
+ continue;
+ }
- g_strfreev(tokens);
- if (err)
- connman_ipaddress_free(*ipaddress);
+ connman_ipaddress_set_p2p(ipaddress, true);
- return err;
+ if (family == AF_INET)
+ ipaddresses->ipaddress_ipv4 = ipaddress;
+ else if (family == AF_INET6)
+ ipaddresses->ipaddress_ipv6 = ipaddress;
+ }
+
+ g_strfreev(addresses);
+
+ return (ipaddresses->ipaddress_ipv4 || ipaddresses->ipaddress_ipv6) ?
+ 0 : -EINVAL;
}
struct ifname_data {
@@ -525,7 +572,7 @@ static int wg_connect(struct vpn_provider *provider,
vpn_provider_connect_cb_t cb,
const char *dbus_sender, void *user_data)
{
- struct connman_ipaddress *ipaddress = NULL;
+ struct wg_ipaddresses ipaddresses = { 0 };
struct wireguard_info *info;
const char *option, *gateway;
char *ifname;
@@ -632,9 +679,9 @@ static int wg_connect(struct vpn_provider *provider,
DBG("Missing WireGuard.Address configuration");
goto error;
}
- err = parse_address(option, gateway, &ipaddress);
+ err = parse_addresses(option, gateway, &ipaddresses);
if (err) {
- DBG("Failed to parse address %s gateway %s", option, gateway);
+ DBG("Failed to parse addresses %s gateway %s", option, gateway);
goto error;
}
@@ -660,14 +707,21 @@ static int wg_connect(struct vpn_provider *provider,
}
vpn_set_ifname(provider, info->device.name);
- if (ipaddress)
- vpn_provider_set_ipaddress(provider, ipaddress);
+
+ if (ipaddresses.ipaddress_ipv4)
+ vpn_provider_set_ipaddress(provider,
+ ipaddresses.ipaddress_ipv4);
+
+ if (ipaddresses.ipaddress_ipv6)
+ vpn_provider_set_ipaddress(provider,
+ ipaddresses.ipaddress_ipv6);
done:
if (cb)
cb(provider, user_data, -err);
- connman_ipaddress_free(ipaddress);
+ connman_ipaddress_free(ipaddresses.ipaddress_ipv4);
+ connman_ipaddress_free(ipaddresses.ipaddress_ipv6);
if (!err)
run_dns_reresolve(info);
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 28/45] inet: Expose __connman_inet_is_any_addr() for plugins to use
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (26 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 27/45] wireguard: Support both IPv4 and IPv6 address Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-08-04 14:57 ` Denis Kenzior
2025-07-11 14:27 ` [PATCH 29/45] wireguard: Set split routing based on AllowedIPs Jussi Laakkonen
` (19 subsequent siblings)
47 siblings, 1 reply; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
---
include/inet.h | 1 +
src/inet.c | 5 +++++
2 files changed, 6 insertions(+)
diff --git a/include/inet.h b/include/inet.h
index 5b2015ed..b7ea0a9b 100644
--- a/include/inet.h
+++ b/include/inet.h
@@ -39,6 +39,7 @@ char *connman_inet_ifname(int index);
int connman_inet_ifup(int index);
int connman_inet_ifdown(int index);
bool connman_inet_is_ifup(int index);
+bool connman_inet_is_any_addr(const char *address, int family);
int connman_inet_set_address(int index, struct connman_ipaddress *ipaddress);
int connman_inet_clear_address(int index, struct connman_ipaddress *ipaddress);
diff --git a/src/inet.c b/src/inet.c
index 54c283ff..e9006e2c 100644
--- a/src/inet.c
+++ b/src/inet.c
@@ -466,6 +466,11 @@ done:
return ret;
}
+bool connman_inet_is_any_addr(const char *address, int family)
+{
+ return __connman_inet_is_any_addr(address, family);
+}
+
struct in6_ifreq {
struct in6_addr ifr6_addr;
__u32 ifr6_prefixlen;
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 28/45] inet: Expose __connman_inet_is_any_addr() for plugins to use
2025-07-11 14:27 ` [PATCH 28/45] inet: Expose __connman_inet_is_any_addr() for plugins to use Jussi Laakkonen
@ 2025-08-04 14:57 ` Denis Kenzior
2025-08-08 12:40 ` Jussi Laakkonen
0 siblings, 1 reply; 69+ messages in thread
From: Denis Kenzior @ 2025-08-04 14:57 UTC (permalink / raw)
To: Jussi Laakkonen, connman
Hi Jussi,
On 7/11/25 9:27 AM, Jussi Laakkonen wrote:
> ---
> include/inet.h | 1 +
> src/inet.c | 5 +++++
> 2 files changed, 6 insertions(+)
>
<snip>
>
> +bool connman_inet_is_any_addr(const char *address, int family)
> +{
> + return __connman_inet_is_any_addr(address, family);
Might as well just rename __connman_inet_is_any_addr and fixup any users to use
__connman_inet_is_any_addr() directly.
> +}
> +
> struct in6_ifreq {
> struct in6_addr ifr6_addr;
> __u32 ifr6_prefixlen;
Regards,
-Denis
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 28/45] inet: Expose __connman_inet_is_any_addr() for plugins to use
2025-08-04 14:57 ` Denis Kenzior
@ 2025-08-08 12:40 ` Jussi Laakkonen
0 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-08-08 12:40 UTC (permalink / raw)
To: Denis Kenzior, connman
Hi Denis,
On 8/4/25 17:57, Denis Kenzior wrote:
> Hi Jussi,
>
> On 7/11/25 9:27 AM, Jussi Laakkonen wrote:
>> ---
>> include/inet.h | 1 +
>> src/inet.c | 5 +++++
>> 2 files changed, 6 insertions(+)
>>
>
> <snip>
>
>> +bool connman_inet_is_any_addr(const char *address, int family)
>> +{
>> + return __connman_inet_is_any_addr(address, family);
>
> Might as well just rename __connman_inet_is_any_addr and fixup any users
> to use __connman_inet_is_any_addr() directly.
It is always an option, yes. Maybe a bit of
less-changes-and-backwards-compatibility has been in my mind always when
doing these.
But if you prefer that way it is no biggie, I can make a V2.
>
>> +}
>> +
>> struct in6_ifreq {
>> struct in6_addr ifr6_addr;
>> __u32 ifr6_prefixlen;
>
> Regards,
> -Denis
BR,
Jussi
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 29/45] wireguard: Set split routing based on AllowedIPs
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (27 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 28/45] inet: Expose __connman_inet_is_any_addr() for plugins to use Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 30/45] Revert "vpn: Remove unused __vpn_provider_check_routes" Jussi Laakkonen
` (18 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Split routing value must be set with WireGuard according to the IP
ranges set with AllowedIPs option. If the option contains an any IP
range (INADDR_ANY/IN6ADDR_ANY_INIT, i.e. 0.0.0.0/0 or ::/0) split
routing must be off since the configuration indicates that the
connection is to be used as the route for all traffic. Similarly,
if there is no any IP range split routing is enabled as only the IP
ranges defined are being encrypted and processed according to the
cryptokey routing table configured on the endpoint.
Also fix a memleak in parse_allowed_ips().
---
vpn/plugins/wireguard.c | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index 2783d9bd..07719273 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -134,7 +134,8 @@ static int parse_key(const char *str, wg_key key)
return 0;
}
-static int parse_allowed_ips(const char *allowed_ips, wg_peer *peer)
+static int parse_allowed_ips(const char *allowed_ips, wg_peer *peer,
+ bool *do_split_routing)
{
struct wg_allowedip *curaip, *allowedip;
char buf[INET6_ADDRSTRLEN];
@@ -142,6 +143,7 @@ static int parse_allowed_ips(const char *allowed_ips, wg_peer *peer)
char *send;
int i;
+ *do_split_routing = true;
curaip = NULL;
tokens = g_strsplit(allowed_ips, ", ", -1);
for (i = 0; tokens[i]; i++) {
@@ -169,6 +171,16 @@ static int parse_allowed_ips(const char *allowed_ips, wg_peer *peer)
allowedip->cidr = g_ascii_strtoull(toks[1], &send, 10);
+ /*
+ * Force split routing off if any address is detected as using
+ * these as allowed IPs indicates that WireGuard is to be used
+ * to route all traffic.
+ */
+ if (connman_inet_is_any_addr(toks[0], allowedip->family))
+ *do_split_routing = false;
+
+ g_strfreev(toks);
+
if (!curaip)
peer->first_allowedip = allowedip;
else
@@ -576,6 +588,7 @@ static int wg_connect(struct vpn_provider *provider,
struct wireguard_info *info;
const char *option, *gateway;
char *ifname;
+ bool do_split_routing = true;
int err = -EINVAL;
info = create_private_data(provider);
@@ -638,12 +651,15 @@ static int wg_connect(struct vpn_provider *provider,
DBG("WireGuard.AllowedIPs is missing");
goto error;
}
- err = parse_allowed_ips(option, &info->peer);
+ err = parse_allowed_ips(option, &info->peer, &do_split_routing);
if (err) {
DBG("Failed to parse allowed IPs %s", option);
goto error;
}
+ vpn_provider_set_boolean(provider, "SplitRouting", do_split_routing,
+ false);
+
option = vpn_provider_get_string(provider,
"WireGuard.PersistentKeepalive");
if (option) {
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 30/45] Revert "vpn: Remove unused __vpn_provider_check_routes"
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (28 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 29/45] wireguard: Set split routing based on AllowedIPs Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 31/45] vpn-provider: Allow to add complete routes and to remove routes Jussi Laakkonen
` (17 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
This reverts commit 192696c5b032b0bdfa09950c43b569131315d490.
---
vpn/vpn-provider.c | 16 ++++++++++++++++
vpn/vpn.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index 7aa72ef0..ff81592c 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -3025,6 +3025,22 @@ void vpn_provider_set_auth_error_limit(struct vpn_provider *provider,
provider->auth_error_limit = limit;
}
+bool __vpn_provider_check_routes(struct vpn_provider *provider)
+{
+ if (!provider)
+ return false;
+
+ if (provider->user_routes &&
+ g_hash_table_size(provider->user_routes) > 0)
+ return true;
+
+ if (provider->routes &&
+ g_hash_table_size(provider->routes) > 0)
+ return true;
+
+ return false;
+}
+
void *vpn_provider_get_data(struct vpn_provider *provider)
{
return provider->driver_data;
diff --git a/vpn/vpn.h b/vpn/vpn.h
index 88afce45..01e9ac11 100644
--- a/vpn/vpn.h
+++ b/vpn/vpn.h
@@ -73,6 +73,7 @@ void __vpn_ipconfig_cleanup(void);
#include "vpn-provider.h"
char *__vpn_provider_create_identifier(const char *host, const char *domain);
+bool __vpn_provider_check_routes(struct vpn_provider *provider);
int __vpn_provider_append_user_route(struct vpn_provider *provider,
int family, const char *network,
const char *netmask, const char *gateway);
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 31/45] vpn-provider: Allow to add complete routes and to remove routes
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (29 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 30/45] Revert "vpn: Remove unused __vpn_provider_check_routes" Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 32/45] wireguard: Add routes for other than any addresses Jussi Laakkonen
` (16 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Adding functionality to add complete routes when all parameters are
known, like with WireGuard. Using the route_env_parse() cb in such case
is too complex and with daemonless VPNs the parameters can be added
once. Move the route lookup into its own function and follow coding
guidelines on memory handling.
Also allow to remove all or a single route from the hash table. In
WireGuard the endpoint may change and this is required for removing the
obsolete routes.
---
vpn/vpn-provider.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++
vpn/vpn-provider.h | 7 ++++++
2 files changed, 61 insertions(+)
diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index ff81592c..7ec21b0b 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -3253,6 +3253,60 @@ int vpn_provider_append_route(struct vpn_provider *provider,
return 0;
}
+int vpn_provider_append_route_complete(struct vpn_provider *provider,
+ unsigned long idx, int family,
+ const char *network, const char *netmask,
+ const char *gateway)
+{
+ struct vpn_route *route;
+
+ DBG("provider %p idx %lu family %d network %s netmask %s gateway %s",
+ provider, idx, family, network,
+ netmask, gateway);
+
+ if (!netmask || !gateway || !network)
+ return -EINVAL;
+
+ if (g_hash_table_lookup(provider->routes, GINT_TO_POINTER(idx)))
+ return -EALREADY;
+
+ route = g_new0(struct vpn_route, 1);
+ route->family = family;
+ route->network = g_strdup(network);
+ route->netmask = g_strdup(netmask);
+ route->gateway = g_strdup(gateway);
+
+ g_hash_table_replace(provider->routes, GINT_TO_POINTER(idx), route);
+ provider_schedule_changed(provider);
+
+ return 0;
+}
+
+void vpn_provider_delete_all_routes(struct vpn_provider *provider)
+{
+ DBG("provider %p", provider);
+;
+ if (!__vpn_provider_check_routes(provider))
+ return;
+
+ g_hash_table_remove_all(provider->routes);
+ provider_schedule_changed(provider);
+}
+
+int vpn_provider_delete_route(struct vpn_provider *provider,
+ unsigned long idx)
+{
+ if (!__vpn_provider_check_routes(provider))
+ return -ENOENT;
+
+ if (!g_hash_table_remove(provider->routes, GINT_TO_POINTER(idx)))
+ return -EINVAL;
+
+ provider_schedule_changed(provider);
+
+ return 0;
+}
+
const char *vpn_provider_get_driver_name(struct vpn_provider *provider)
{
if (!provider->driver)
diff --git a/vpn/vpn-provider.h b/vpn/vpn-provider.h
index 8a8b6bfd..c7487525 100644
--- a/vpn/vpn-provider.h
+++ b/vpn/vpn-provider.h
@@ -124,6 +124,13 @@ int vpn_provider_set_nameservers(struct vpn_provider *provider,
const char *nameservers);
int vpn_provider_append_route(struct vpn_provider *provider,
const char *key, const char *value);
+int vpn_provider_append_route_complete(struct vpn_provider *provider,
+ unsigned long idx, int family,
+ const char *address, const char *netmask,
+ const char *gateway);
+void vpn_provider_delete_all_routes(struct vpn_provider *provider);
+int vpn_provider_delete_route(struct vpn_provider *provider,
+ unsigned long index);
const char *vpn_provider_get_driver_name(struct vpn_provider *provider);
const char *vpn_provider_get_save_group(struct vpn_provider *provider);
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 32/45] wireguard: Add routes for other than any addresses
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (30 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 31/45] vpn-provider: Allow to add complete routes and to remove routes Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 33/45] wireguard: Fix string list parsing and IP tunneling Jussi Laakkonen
` (15 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
This adds a use of the vpn_provider_append_route_complete() to setup the
routes to the AllowedIP networks after WireGuard has connected. Peer
endpoint is used as a gateway and routes to other address family types
are ignored, as well as the any routes, which are setup by
libwireguard.c. This follows then the approach used in wg-quick.
When the endpoint is resolved to a new address the old routes are
removed and new ones are added. This is done without a delay, at startup
there is ROUTE_SETUP_TIMEOUT delay (200ms) to make sure setup is complete.
TODO: do not ignore routes if there is IP address set for the families.
---
vpn/plugins/wireguard.c | 149 +++++++++++++++++++++++++++++++++++++---
1 file changed, 139 insertions(+), 10 deletions(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index 07719273..a22402c8 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -53,6 +53,7 @@
#define DNS_RERESOLVE_TIMEOUT 20
#define DNS_RERESOLVE_ERROR_LIMIT 5
+#define ROUTE_SETUP_TIMEOUT 200 // ms
#define ARRAY_SIZE(a) (sizeof(a)/sizeof(a[0]))
struct wireguard_info {
@@ -66,6 +67,7 @@ struct wireguard_info {
guint resolv_id;
guint remove_resolv_id;
guint dying_id;
+ guint route_setup_id;
};
struct sockaddr_u {
@@ -262,6 +264,25 @@ struct wg_ipaddresses {
struct connman_ipaddress *ipaddress_ipv6;
};
+static char *cidr_to_netmask(int family, unsigned char cidr)
+{
+ switch (family) {
+ case AF_INET:
+ return g_strdup_printf("%d.%d.%d.%d",
+ ((0xffffffff << (32 - cidr)) >> 24) & 0xff,
+ ((0xffffffff << (32 - cidr)) >> 16) & 0xff,
+ ((0xffffffff << (32 - cidr)) >> 8) & 0xff,
+ ((0xffffffff << (32 - cidr)) >> 0) & 0xff);
+
+ case AF_INET6:
+ return g_strdup_printf("%u", cidr);
+ default:
+ break;
+ }
+
+ return NULL;
+}
+
static int parse_addresses(const char *address, const char *gateway,
struct wg_ipaddresses *ipaddresses)
{
@@ -303,12 +324,7 @@ static int parse_addresses(const char *address, const char *gateway,
}
family = AF_INET;
- netmask = g_strdup_printf("%d.%d.%d.%d",
- ((0xffffffff << (32 - prefixlen)) >> 24) & 0xff,
- ((0xffffffff << (32 - prefixlen)) >> 16) & 0xff,
- ((0xffffffff << (32 - prefixlen)) >> 8) & 0xff,
- ((0xffffffff << (32 - prefixlen)) >> 0) & 0xff);
-
+ netmask = cidr_to_netmask(family, prefixlen);
ipaddress = connman_ipaddress_alloc(family);
err = connman_ipaddress_set_ipv4(ipaddress, tokens[0],
netmask, gateway);
@@ -401,6 +417,7 @@ static bool sockaddr_cmp_addr(struct sockaddr_u *a, struct sockaddr_u *b)
}
static void run_dns_reresolve(struct wireguard_info *info);
+static void run_route_setup(struct wireguard_info *info, guint timeout);
static void remove_resolv(struct wireguard_info *info)
{
@@ -524,6 +541,13 @@ static void resolve_endpoint_cb(GResolvResultStatus status,
info->device.name);
run_dns_reresolve(info);
+
+ /*
+ * Endpoint has changed and only one peer is used -> all old routes
+ * are invalid. Redo them without delay.
+ */
+ vpn_provider_delete_all_routes(info->provider);
+ run_route_setup(info, 0);
}
static int disconnect(struct vpn_provider *provider, int error);
@@ -562,16 +586,107 @@ static gboolean wg_dns_reresolve_cb(gpointer user_data)
return G_SOURCE_REMOVE;
}
+static const char *endpoint_to_str(struct wg_peer *peer, char *buf,
+ socklen_t len)
+{
+ struct sockaddr_u *addr;
+ int family;
+
+ addr = (struct sockaddr_u *)&peer->endpoint.addr;
+ family = peer->endpoint.addr.sa_family;
+
+ switch (family) {
+ case AF_INET:
+ return inet_ntop(family, &addr->sin.sin_addr, buf, len);
+ case AF_INET6:
+ return inet_ntop(family, &addr->sin6.sin6_addr, buf, len);
+ default:
+ break;
+ }
+
+ return NULL;
+}
+
+static gboolean wg_route_setup_cb(gpointer user_data)
+{
+ struct wireguard_info *info = user_data;
+ struct wg_allowedip *allowedip;
+ char addr[INET6_ADDRSTRLEN] = { 0 };
+ char endpoint[INET6_ADDRSTRLEN] = { 0 };
+ char *netmask;
+ int family;
+ unsigned long idx = 0;
+
+ info->route_setup_id = 0;
+
+ family = info->peer.endpoint.addr.sa_family;
+
+ if (!endpoint_to_str(&info->peer, endpoint, INET6_ADDRSTRLEN)) {
+ DBG("Faulty endpoint set, use endpoint_fqdn");
+ memcpy(&endpoint, info->endpoint_fqdn, INET6_ADDRSTRLEN);
+ family = connman_inet_check_ipaddress(info->endpoint_fqdn);
+ }
+
+ wg_for_each_allowedip(&info->peer, allowedip) {
+ // TODO: search peers when multiple peers are supported
+ if (allowedip->family != family) {
+ DBG("Ignoring AllowedIP with different family as host");
+ continue;
+ }
+
+ memset(&addr, 0, INET6_ADDRSTRLEN);
+
+ switch (allowedip->family) {
+ case AF_INET:
+ if (!inet_ntop(allowedip->family, &allowedip->ip4, addr,
+ INET6_ADDRSTRLEN)) {
+ DBG("ignore invalid IPv4 address");
+ continue;
+ }
+
+ break;
+ case AF_INET6:
+ if (!inet_ntop(allowedip->family, &allowedip->ip6, addr,
+ INET6_ADDRSTRLEN)) {
+ DBG("ignore invalid IPv6 address");
+ continue;
+ }
+
+ break;
+ default:
+ DBG("ignore invalid IP family");
+ continue;
+ }
+
+ if (connman_inet_is_any_addr(addr, allowedip->family)) {
+ DBG("ignore any addr %s", addr);
+ continue;
+ }
+
+ netmask = cidr_to_netmask(allowedip->family, allowedip->cidr);
+
+ vpn_provider_append_route_complete(info->provider, idx,
+ allowedip->family, addr,
+ netmask, endpoint);
+
+ g_free(netmask);
+ ++idx;
+ }
+
+ return G_SOURCE_REMOVE;
+}
+
static void run_dns_reresolve(struct wireguard_info *info)
{
if (info->reresolve_id)
g_source_remove(info->reresolve_id);
+ info->reresolve_id = 0;
+
if (vpn_provider_get_connection_errors(info->provider) >=
DNS_RERESOLVE_ERROR_LIMIT) {
connman_warn("reresolve error limit reached");
disconnect(info->provider, -ENONET);
- info->reresolve_id = 0;
return;
}
@@ -579,6 +694,14 @@ static void run_dns_reresolve(struct wireguard_info *info)
wg_dns_reresolve_cb, info);
}
+static void run_route_setup(struct wireguard_info *info, guint timeout)
+{
+ if (info->route_setup_id)
+ g_source_remove(info->route_setup_id);
+
+ info->route_setup_id = g_timeout_add(timeout, wg_route_setup_cb, info);
+}
+
static int wg_connect(struct vpn_provider *provider,
struct connman_task *task, const char *if_name,
vpn_provider_connect_cb_t cb,
@@ -660,6 +783,7 @@ static int wg_connect(struct vpn_provider *provider,
vpn_provider_set_boolean(provider, "SplitRouting", do_split_routing,
false);
+
option = vpn_provider_get_string(provider,
"WireGuard.PersistentKeepalive");
if (option) {
@@ -739,8 +863,10 @@ done:
connman_ipaddress_free(ipaddresses.ipaddress_ipv4);
connman_ipaddress_free(ipaddresses.ipaddress_ipv6);
- if (!err)
+ if (!err) {
run_dns_reresolve(info);
+ run_route_setup(info, ROUTE_SETUP_TIMEOUT);
+ }
return err;
@@ -795,9 +921,12 @@ static int disconnect(struct vpn_provider *provider, int err)
if (info->dying_id)
return -EALREADY;
- if (info->reresolve_id > 0)
+ if (info->reresolve_id)
g_source_remove(info->reresolve_id);
+ if (info->route_setup_id)
+ g_source_remove(info->route_setup_id);
+
if (info->resolv || info->resolv_id)
remove_resolv(info);
@@ -871,7 +1000,7 @@ static struct vpn_driver vpn_driver = {
.disconnect = wg_disconnect,
.save = wg_save,
.error_code = wg_error_code,
- .uses_vpn_agent = wg_uses_vpn_agent
+ .uses_vpn_agent = wg_uses_vpn_agent
};
static int wg_init(void)
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 33/45] wireguard: Fix string list parsing and IP tunneling
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (31 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 32/45] wireguard: Add routes for other than any addresses Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 34/45] wireguard: Treat initial connect failure as unreachable host Jussi Laakkonen
` (14 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
The string lists read, for example, Mullvad config files do not contain
spaces, use g_strsplit_set() for this.
Prevent adding only the any routes for the current peer family to avoid
duplicates. This fixes dual protocol tunneling on a single WireGuard
peer, such as IPv4 server with both IPv4 and IPv6 addresses.
---
vpn/plugins/wireguard.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index a22402c8..ee7149b7 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -147,7 +147,7 @@ static int parse_allowed_ips(const char *allowed_ips, wg_peer *peer,
*do_split_routing = true;
curaip = NULL;
- tokens = g_strsplit(allowed_ips, ", ", -1);
+ tokens = g_strsplit_set(allowed_ips, ", ", -1);
for (i = 0; tokens[i]; i++) {
toks = g_strsplit(tokens[i], "/", -1);
if (g_strv_length(toks) != 2) {
@@ -294,7 +294,7 @@ static int parse_addresses(const char *address, const char *gateway,
int err;
int i;
- addresses = g_strsplit(address, ", ", -1);
+ addresses = g_strsplit_set(address, ", ", -1);
if (!g_strv_length(addresses)) {
g_strfreev(addresses);
return -EINVAL;
@@ -628,12 +628,6 @@ static gboolean wg_route_setup_cb(gpointer user_data)
}
wg_for_each_allowedip(&info->peer, allowedip) {
- // TODO: search peers when multiple peers are supported
- if (allowedip->family != family) {
- DBG("Ignoring AllowedIP with different family as host");
- continue;
- }
-
memset(&addr, 0, INET6_ADDRSTRLEN);
switch (allowedip->family) {
@@ -658,7 +652,9 @@ static gboolean wg_route_setup_cb(gpointer user_data)
continue;
}
- if (connman_inet_is_any_addr(addr, allowedip->family)) {
+ /* Ignore any routes for this peer family to avoid duplicates */
+ if (connman_inet_is_any_addr(addr, allowedip->family) &&
+ allowedip->family == family) {
DBG("ignore any addr %s", addr);
continue;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 34/45] wireguard: Treat initial connect failure as unreachable host
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (32 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 33/45] wireguard: Fix string list parsing and IP tunneling Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 35/45] service: handle also EALREADY in service_connect() Jussi Laakkonen
` (13 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
When changing networks WireGuard may connect too early and the network
is not setup yet, causing connection attempts to the resolved host to
fail. These should not be treated as a login error, which is a terminal
error for a VPN. Instead, treat these cases as unreachable host errors
which result in an increase in the connection error limit and keep the
autoconnect value untouched.
Also add a bit more debug into the plugin. This will help to identify
configuration errors in the future.
---
vpn/plugins/wireguard.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index ee7149b7..756c6f88 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -230,7 +230,8 @@ static int parse_endpoint(const char *host, const char *port, struct sockaddr_u
err = getaddrinfo(tokens[0], port, &hints, &result);
g_strfreev(tokens);
- if (err < 0) {
+ /* Any non-zero return from getaddrinfo is an error */
+ if (err) {
DBG("Failed to resolve host address: %s", gai_strerror(err));
return -EINVAL;
}
@@ -249,8 +250,9 @@ static int parse_endpoint(const char *host, const char *port, struct sockaddr_u
}
if (!rp) {
+ DBG("no connectable address found in results");
freeaddrinfo(result);
- return -EINVAL;
+ return -EHOSTUNREACH;
}
memcpy(addr, rp->ai_addr, rp->ai_addrlen);
@@ -572,6 +574,7 @@ static gboolean wg_dns_reresolve_cb(gpointer user_data)
return G_SOURCE_REMOVE;
}
+ DBG("endpoint_fqdn %s", info->endpoint_fqdn);
info->resolv_id = vpn_util_resolve_hostname(info->resolv,
info->endpoint_fqdn,
resolve_endpoint_cb, info);
@@ -872,8 +875,15 @@ error:
* looping when parameters are incorrect and VPN stays in failed
* state.
*/
- vpn_provider_add_error(provider, VPN_PROVIDER_ERROR_LOGIN_FAILED);
- err = -ECONNABORTED;
+ if (err == -EHOSTUNREACH) {
+ vpn_provider_add_error(provider,
+ VPN_PROVIDER_ERROR_CONNECT_FAILED);
+ } else {
+ vpn_provider_add_error(provider,
+ VPN_PROVIDER_ERROR_LOGIN_FAILED);
+ err = -ECONNABORTED;
+ }
+
goto done;
}
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 35/45] service: handle also EALREADY in service_connect()
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (33 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 34/45] wireguard: Treat initial connect failure as unreachable host Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 36/45] vpn-provider: Make daemonless VPNs to connect when connmand is online Jussi Laakkonen
` (12 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
---
src/service.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/src/service.c b/src/service.c
index 9fd8fdb1..28e6192c 100644
--- a/src/service.c
+++ b/src/service.c
@@ -9945,16 +9945,19 @@ static int service_connect(struct connman_service *service)
else
return -EOPNOTSUPP;
- if (err < 0) {
- if (err != -EINPROGRESS) {
- __connman_service_ipconfig_indicate_state(service,
+ switch (err) {
+ case 0:
+ case -EALREADY:
+ case -EINPROGRESS:
+ break;
+ default:
+ __connman_service_ipconfig_indicate_state(service,
CONNMAN_SERVICE_STATE_FAILURE,
CONNMAN_IPCONFIG_TYPE_IPV4);
- __connman_service_ipconfig_indicate_state(service,
+ __connman_service_ipconfig_indicate_state(service,
CONNMAN_SERVICE_STATE_FAILURE,
CONNMAN_IPCONFIG_TYPE_IPV6);
- __connman_stats_service_unregister(service);
- }
+ __connman_stats_service_unregister(service);
}
return err;
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 36/45] vpn-provider: Make daemonless VPNs to connect when connmand is online
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (34 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 35/45] service: handle also EALREADY in service_connect() Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 37/45] vpn: Implement getter for the flags set by the VPN Jussi Laakkonen
` (11 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Reformat the connmand status checking code to account the exact online
state. Add a function get_flags for vpn_provider_driver struct to get
the flags set by the plugin to determine whether VPN can be attempted to
connect in ready or online state. Do connect daemonless VPNs only after
connmand reports being online, as they may be connected too early and
network is not setup yet if they get connected when connmand has reached
ready state. Thus, this does not change the functionality for the VPNs
using a daemon.
---
vpn/vpn-provider.c | 67 +++++++++++++++++++++++++++++++++-------------
vpn/vpn-provider.h | 1 +
2 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index 7ec21b0b..9211368a 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -125,7 +125,14 @@ static unsigned int get_connman_state_timeout;
static guint connman_signal_watch;
static guint connman_service_watch;
-static bool connman_online;
+enum connman_state {
+ CONNMAN_IDLE = 0,
+ CONNMAN_OFFLINE,
+ CONNMAN_READY,
+ CONNMAN_ONLINE
+};
+
+static enum connman_state connman_online_state = CONNMAN_IDLE;
static bool state_query_completed;
@@ -164,22 +171,46 @@ static void set_state(const char *new_state)
if (!new_state || !*new_state)
return;
- DBG("old state %s new state %s",
- connman_online ?
- CONNMAN_STATE_ONLINE "/" CONNMAN_STATE_READY :
- CONNMAN_STATE_OFFLINE "/" CONNMAN_STATE_IDLE,
- new_state);
-
- /* States "online" and "ready" mean connman is online */
- if (!g_ascii_strcasecmp(new_state, CONNMAN_STATE_ONLINE) ||
- !g_ascii_strcasecmp(new_state, CONNMAN_STATE_READY))
- connman_online = true;
- /* Everything else means connman is offline */
+ DBG("received %s, old state %d", new_state, connman_online_state);
+
+ if (!g_ascii_strcasecmp(new_state, CONNMAN_STATE_ONLINE))
+ connman_online_state = CONNMAN_ONLINE;
+ else if (!g_ascii_strcasecmp(new_state, CONNMAN_STATE_READY))
+ connman_online_state = CONNMAN_READY;
+ else if (!g_ascii_strcasecmp(new_state, CONNMAN_STATE_OFFLINE))
+ connman_online_state = CONNMAN_OFFLINE;
else
- connman_online = false;
+ connman_online_state = CONNMAN_IDLE;
+
+ DBG("new state %d ", connman_online_state);
+}
+
+static bool is_connman_connected(struct vpn_provider *provider)
+{
+ int flags = 0;
+
+ DBG("provider %p connmand state %d", provider, connman_online_state);
+
+ if (provider && provider->driver && provider->driver->get_flags)
+ flags = provider->driver->get_flags(provider);
- DBG("set state %s connman_online=%s ", new_state,
- connman_online ? "true" : "false");
+ switch (connman_online_state) {
+ case CONNMAN_IDLE:
+ case CONNMAN_OFFLINE:
+ return false;
+ case CONNMAN_READY:
+ /* VPNs without daemon may require that network is setup. */
+ if (flags & VPN_FLAG_NO_DAEMON) {
+ DBG("daemonless provider, return false in ready");
+ return false;
+ }
+
+ /* fall-through */
+ case CONNMAN_ONLINE:
+ break;
+ }
+
+ return true;
}
static void free_route(gpointer data)
@@ -832,8 +863,8 @@ static gboolean do_connect_timeout_function(gpointer data)
DBG("");
- /* Keep in main loop if connman is not online. */
- if (!connman_online)
+ /* Keep in main loop if connman is not ready for this VPN. */
+ if (!is_connman_connected(provider))
return G_SOURCE_CONTINUE;
provider->do_connect_timeout = 0;
@@ -887,7 +918,7 @@ static DBusMessage *do_connect(DBusConnection *conn, DBusMessage *msg,
DBG("conn %p provider %p", conn, provider);
- if (!connman_online) {
+ if (!is_connman_connected(provider)) {
if (state_query_completed) {
DBG("%s not started - ConnMan not online/ready",
provider->identifier);
diff --git a/vpn/vpn-provider.h b/vpn/vpn-provider.h
index c7487525..a7ec5109 100644
--- a/vpn/vpn-provider.h
+++ b/vpn/vpn-provider.h
@@ -175,6 +175,7 @@ struct vpn_provider_driver {
int *family, unsigned long *idx,
enum vpn_provider_route_type *type);
bool (*uses_vpn_agent) (struct vpn_provider *provider);
+ int (*get_flags)(struct vpn_provider *provider);
};
int vpn_provider_driver_register(struct vpn_provider_driver *driver);
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 37/45] vpn: Implement getter for the flags set by the VPN
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (35 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 36/45] vpn-provider: Make daemonless VPNs to connect when connmand is online Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 38/45] vpn-provider: Delay connect of daemonless VPNs until connmand is online Jussi Laakkonen
` (10 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Allow the provider to get the flags set by the VPN plugin. Add this to
the updated provider_driver struct.
---
vpn/plugins/vpn.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/vpn/plugins/vpn.c b/vpn/plugins/vpn.c
index a6b738b0..d06473d9 100644
--- a/vpn/plugins/vpn.c
+++ b/vpn/plugins/vpn.c
@@ -830,6 +830,22 @@ static bool vpn_uses_vpn_agent(struct vpn_provider *provider)
return true;
}
+static int vpn_get_flags(struct vpn_provider *provider)
+{
+ struct vpn_driver_data *vpn_driver_data = NULL;
+ const char *name = NULL;
+
+ if (!provider)
+ return 0;
+
+ name = vpn_provider_get_driver_name(provider);
+ vpn_driver_data = g_hash_table_lookup(driver_hash, name);
+ if (vpn_driver_data)
+ return vpn_driver_data->vpn_driver->flags;
+
+ return 0;
+}
+
int vpn_register(const char *name, const struct vpn_driver *vpn_driver,
const char *program)
{
@@ -856,6 +872,7 @@ int vpn_register(const char *name, const struct vpn_driver *vpn_driver,
data->provider_driver.set_state = vpn_set_state;
data->provider_driver.route_env_parse = vpn_route_env_parse;
data->provider_driver.uses_vpn_agent = vpn_uses_vpn_agent;
+ data->provider_driver.get_flags = vpn_get_flags;
if (!driver_hash)
driver_hash = g_hash_table_new_full(g_str_hash,
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 38/45] vpn-provider: Delay connect of daemonless VPNs until connmand is online
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (36 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 37/45] vpn: Implement getter for the flags set by the VPN Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 39/45] wireguard: Rework hostname resolve, split code and do not resolve IP Jussi Laakkonen
` (9 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
When the connmand state has been queried and a daemonless VPN is being
connected it must be connected later when online state is reached. This
ensures that if the online check on connmand transport is slow, the
daemonless VPN will get connected when state changes.
---
vpn/vpn-provider.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index 9211368a..f72030af 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -920,6 +920,15 @@ static DBusMessage *do_connect(DBusConnection *conn, DBusMessage *msg,
if (!is_connman_connected(provider)) {
if (state_query_completed) {
+ /* Query done but connmand not online for daemonless */
+ if (connman_online_state >= CONNMAN_READY) {
+ DBG("Provider %s start delayed, wait for "
+ "connman online state",
+ provider->identifier);
+ do_connect_later(provider, conn, msg);
+ return NULL;
+ }
+
DBG("%s not started - ConnMan not online/ready",
provider->identifier);
return __connman_error_failed(msg, ENOLINK);
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 39/45] wireguard: Rework hostname resolve, split code and do not resolve IP
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (37 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 38/45] vpn-provider: Delay connect of daemonless VPNs until connmand is online Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 40/45] vpn-util: Add wrapper for adding a namerver for GResolv Jussi Laakkonen
` (8 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
This changes how the resolving of the hostname is done and restructures
the code.
First, the endpoint parsing is split into two, mainly because the
resolving of the host is done differently when doing initial connect,
and when executing the DNS reresolve. This is because initial resolving
can be done with getaddrinfo() but reresolve is done with GResolv to
avoid blocking in case the network is not properly setup, which leads
vpnd to be frozen until the timeout occurs. With GResolv the results are
simply converted to sockaddr struct(s) using getifaddrs() with
AI_NUMERICHOST to avoid another DNS query.
Second, if the host is defined as an IP address there is no need to
start the DNS reresolve.
Third, the stop condition for DNS reresolve does now retry also with
name error (NXDOMAIN) and no answer (NOERROR without a message).
---
vpn/plugins/wireguard.c | 119 ++++++++++++++++++++++++++++------------
1 file changed, 84 insertions(+), 35 deletions(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index 756c6f88..a2d92ec6 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -197,42 +197,27 @@ static int parse_allowed_ips(const char *allowed_ips, wg_peer *peer,
return 0;
}
-static int parse_endpoint(const char *host, const char *port, struct sockaddr_u *addr)
+static int get_endpoint_addr(const char *host, const char *port, int flags,
+ struct sockaddr_u *addr)
{
struct addrinfo hints;
- struct addrinfo *result, *rp;
- char **tokens;
+ struct addrinfo *result = NULL, *rp;
int sk;
int err;
- unsigned int len;
-
- /*
- * getaddrinfo() relies on inet_pton() that suggests using addresses
- * without CIDR notation. Host should contain the address in CIDR
- * notation to be able to pass the prefix length to ConnMan via D-Bus.
- */
- tokens = g_strsplit(host, "/", -1);
- len = g_strv_length(tokens);
- if (len > 2 || len < 1) {
- DBG("Failure tokenizing host %s", host);
- g_strfreev(tokens);
- return -EINVAL;
- }
-
- DBG("using host %s", tokens[0]);
memset(&hints, 0, sizeof(struct addrinfo));
hints.ai_family = AF_UNSPEC;
hints.ai_socktype = SOCK_DGRAM;
- hints.ai_flags = 0;
+ hints.ai_flags = flags;
hints.ai_protocol = 0;
- err = getaddrinfo(tokens[0], port, &hints, &result);
- g_strfreev(tokens);
-
- /* Any non-zero return from getaddrinfo is an error */
- if (err) {
+ err = getaddrinfo(host, port, &hints, &result);
+ if (err) { /* Any non-zero return from getaddrinfo is an error */
DBG("Failed to resolve host address: %s", gai_strerror(err));
+
+ if (result)
+ freeaddrinfo(result);
+
return -EINVAL;
}
@@ -250,7 +235,8 @@ static int parse_endpoint(const char *host, const char *port, struct sockaddr_u
}
if (!rp) {
- DBG("no connectable address found in results");
+ DBG("no connectable address found in results: %s",
+ strerror(errno));
freeaddrinfo(result);
return -EHOSTUNREACH;
}
@@ -261,6 +247,62 @@ static int parse_endpoint(const char *host, const char *port, struct sockaddr_u
return 0;
}
+static int parse_endpoint_hostname(const char *host, const char *port,
+ struct sockaddr_u *addr)
+{
+ char **tokens;
+ unsigned int len;
+ int err;
+
+ /*
+ * getaddrinfo() relies on inet_pton() that suggests using addresses
+ * without CIDR notation. Host should contain the address in CIDR
+ * notation to be able to pass the prefix length to ConnMan via D-Bus.
+ */
+ tokens = g_strsplit(host, "/", -1);
+ len = g_strv_length(tokens);
+ if (len > 2 || len < 1) {
+ DBG("Failure tokenizing host %s", host);
+ g_strfreev(tokens);
+ return -EINVAL;
+ }
+
+ DBG("using host %s", tokens[0]);
+
+ err = get_endpoint_addr(tokens[0], port, 0, addr);
+ if (!err)
+ DBG("success");
+
+ g_strfreev(tokens);
+
+ return err;
+}
+
+static int parse_endpoint_results(char **results, const char *port,
+ struct sockaddr_u *addr)
+{
+ int err = 0;
+ int i;
+
+ if (!results) {
+ DBG("no results");
+ return -EINVAL;
+ }
+
+ for (i = 0; results[i]; i++) {
+ DBG("using host %s", results[i]);
+
+ /* Use getaddrinfo to fill in the structs after resolve */
+ err = get_endpoint_addr(results[i], port, AI_NUMERICHOST, addr);
+ if (!err) {
+ DBG("success");
+ return err;
+ }
+ }
+
+ return err;
+}
+
struct wg_ipaddresses {
struct connman_ipaddress *ipaddress_ipv4;
struct connman_ipaddress *ipaddress_ipv6;
@@ -454,7 +496,7 @@ static void resolve_endpoint_cb(GResolvResultStatus status,
{
struct wireguard_info *info = user_data;
struct sockaddr_u addr;
- int err;
+ int err = 0;
DBG("");
@@ -486,6 +528,8 @@ static void resolve_endpoint_cb(GResolvResultStatus status,
DBG("resolv success, parse endpoint");
break;
/* request timeouts or an server issue is not an error, try again */
+ case G_RESOLV_RESULT_STATUS_NAME_ERROR: /* NXDOMAIN, might recover? */
+ case G_RESOLV_RESULT_STATUS_NO_ANSWER:
case G_RESOLV_RESULT_STATUS_NO_RESPONSE:
case G_RESOLV_RESULT_STATUS_SERVER_FAILURE:
DBG("retry DNS reresolve");
@@ -498,15 +542,12 @@ static void resolve_endpoint_cb(GResolvResultStatus status,
/* Consider these as non-continuable errors */
case G_RESOLV_RESULT_STATUS_ERROR:
case G_RESOLV_RESULT_STATUS_FORMAT_ERROR:
- case G_RESOLV_RESULT_STATUS_NAME_ERROR:
case G_RESOLV_RESULT_STATUS_NOT_IMPLEMENTED:
case G_RESOLV_RESULT_STATUS_REFUSED:
- case G_RESOLV_RESULT_STATUS_NO_ANSWER:
- DBG("stop DNS reresolve");
- if (info->provider)
+ DBG("stop DNS reresolve, error %d", status);
+ if (err && info->provider)
vpn_provider_add_error(info->provider,
VPN_PROVIDER_ERROR_CONNECT_FAILED);
-
return;
}
@@ -514,7 +555,7 @@ static void resolve_endpoint_cb(GResolvResultStatus status,
* If this fails after being connected it means configuration error
* that results in connection errors.
*/
- err = parse_endpoint(info->endpoint_fqdn, info->port, &addr);
+ err = parse_endpoint_results(results, info->port, &addr);
if (err) {
if (info->provider)
vpn_provider_add_error(info->provider,
@@ -575,6 +616,7 @@ static gboolean wg_dns_reresolve_cb(gpointer user_data)
}
DBG("endpoint_fqdn %s", info->endpoint_fqdn);
+
info->resolv_id = vpn_util_resolve_hostname(info->resolv,
info->endpoint_fqdn,
resolve_endpoint_cb, info);
@@ -712,6 +754,7 @@ static int wg_connect(struct vpn_provider *provider,
char *ifname;
bool do_split_routing = true;
int err = -EINVAL;
+ int family;
info = create_private_data(provider);
@@ -803,7 +846,7 @@ static int wg_connect(struct vpn_provider *provider,
* is needed as it is done over potentially misconfigured WireGuard
* connection that may end up blocking vpnd with getaddrinfo().
*/
- err = parse_endpoint(gateway, option,
+ err = parse_endpoint_hostname(gateway, option,
(struct sockaddr_u *)&info->peer.endpoint.addr);
if (err) {
DBG("Failed to parse endpoint %s:%s", gateway, option);
@@ -863,7 +906,13 @@ done:
connman_ipaddress_free(ipaddresses.ipaddress_ipv6);
if (!err) {
- run_dns_reresolve(info);
+ /* Run DNS reresolve only for hostnames that require resolve. */
+ family = connman_inet_check_ipaddress(info->endpoint_fqdn);
+ if (family != AF_INET && family != AF_INET6) {
+ DBG("start DNS reresolve for %s", info->endpoint_fqdn);
+ run_dns_reresolve(info);
+ }
+
run_route_setup(info, ROUTE_SETUP_TIMEOUT);
}
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 40/45] vpn-util: Add wrapper for adding a namerver for GResolv
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (38 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 39/45] wireguard: Rework hostname resolve, split code and do not resolve IP Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 41/45] service: Send the DNS servers of VPN's transport when VPN is ready Jussi Laakkonen
` (7 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
The nameservers must be added for the GResolv for the resolving to
succeed. Add a wrapper for this.
---
vpn/vpn-util.c | 8 ++++++++
vpn/vpn.h | 2 ++
2 files changed, 10 insertions(+)
diff --git a/vpn/vpn-util.c b/vpn/vpn-util.c
index 2e526701..c1264935 100644
--- a/vpn/vpn-util.c
+++ b/vpn/vpn-util.c
@@ -222,6 +222,14 @@ out:
return err;
}
+bool vpn_util_resolv_add_nameserver(GResolv *resolv, const char *address,
+ uint16_t port, unsigned long flags)
+{
+ DBG("dns %s:%u", address, port);
+
+ return g_resolv_add_nameserver(resolv, address, port, flags);
+}
+
unsigned int vpn_util_resolve_hostname(GResolv *resolv,
const char *hostname, GResolvResultFunc func,
gpointer user_data)
diff --git a/vpn/vpn.h b/vpn/vpn.h
index 01e9ac11..869a3c2c 100644
--- a/vpn/vpn.h
+++ b/vpn/vpn.h
@@ -138,6 +138,8 @@ int vpn_util_create_path(const char *path, uid_t uid, gid_t grp, int mode);
#include <gweb/gresolv.h>
+bool vpn_util_resolv_add_nameserver(GResolv *resolv, const char *address,
+ uint16_t port, unsigned long flags);
unsigned int vpn_util_resolve_hostname(GResolv *resolv,
const char *hostname, GResolvResultFunc func,
gpointer user_data);
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 41/45] service: Send the DNS servers of VPN's transport when VPN is ready
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (39 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 40/45] vpn-util: Add wrapper for adding a namerver for GResolv Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 42/45] vpn-provider: Add support for set/get "TransportNameservers" Jussi Laakkonen
` (6 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Add a new property to be sent from the VPN service's path when the VPN
reports being ready. This is sent similar to other DNS service lists
when they change but with name "TransportNameservers".
The reason for doing this is that WireGuard does the periodic DNS
reresolve after getting connected, and the reresolving needs to be
done using the DNS server(s) of the transport service, and not the ones
WireGuard has set up.
---
src/service.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/src/service.c b/src/service.c
index 28e6192c..eabd009d 100644
--- a/src/service.c
+++ b/src/service.c
@@ -4929,6 +4929,30 @@ static void dns_configuration_changed(struct connman_service *service)
dns_changed(service);
}
+static void vpn_transport_dns_changed(struct connman_service *service)
+{
+ struct connman_service *transport;
+ const char *ident;
+
+ if (!service || service->type != CONNMAN_SERVICE_TYPE_VPN)
+ return;
+
+ ident = connman_provider_get_string(service->provider, "Transport");
+ if (!ident)
+ return;
+
+ transport = connman_service_lookup_from_identifier(ident);
+ if (!transport)
+ return;
+
+ DBG("transport %p/%s", transport, ident);
+
+ connman_dbus_property_changed_array(service->path,
+ CONNMAN_SERVICE_INTERFACE,
+ "TransportNameservers",
+ DBUS_TYPE_STRING, append_dns, transport);
+}
+
static void domain_changed(struct connman_service *service)
{
if (!allow_property_changed(service))
@@ -9396,6 +9420,9 @@ static int service_indicate_state(struct connman_service *service)
"WiFi.UseWPS", false);
}
+ if (service->type == CONNMAN_SERVICE_TYPE_VPN)
+ vpn_transport_dns_changed(service);
+
gettimeofday(&service->modified, NULL);
service_save(service);
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 42/45] vpn-provider: Add support for set/get "TransportNameservers"
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (40 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 41/45] service: Send the DNS servers of VPN's transport when VPN is ready Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 43/45] wireguard: Add option for using transport nameservers for DNS reresolve Jussi Laakkonen
` (5 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Set the transport nameservers when property change is received as a
D-Bus signal from connmand. The code to get the nameservers is identical
to plugins/vpn.c:extract_nameservers(). And add a generic getter for
plugins to use.
---
vpn/vpn-provider.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++
vpn/vpn-provider.h | 2 ++
2 files changed, 50 insertions(+)
diff --git a/vpn/vpn-provider.c b/vpn/vpn-provider.c
index f72030af..d3affc0e 100644
--- a/vpn/vpn-provider.c
+++ b/vpn/vpn-provider.c
@@ -112,6 +112,7 @@ struct vpn_provider {
unsigned int signal_watch;
unsigned int auth_error_limit;
time_t previous_connect_time;
+ char **transport_nameservers;
};
struct vpn_provider_connect_data {
@@ -1504,6 +1505,7 @@ static void provider_destruct(struct vpn_provider *provider)
g_free(provider->config_entry);
connman_ipaddress_free(provider->prev_ipv4_addr);
connman_ipaddress_free(provider->prev_ipv6_addr);
+ g_strfreev(provider->transport_nameservers);
g_free(provider);
}
@@ -2243,6 +2245,41 @@ static gboolean provider_property_changed(DBusConnection *conn,
*/
vpn_provider_set_boolean(provider, "SplitRouting",
split_routing, true);
+ } else if (g_str_equal(key, "TransportNameservers")) {
+ DBusMessageIter entry;
+ char **nameservers = NULL;
+ int i = 0;
+
+ DBG("TransportNameservers");
+
+ dbus_message_iter_recurse(&value, &entry);
+
+ while (dbus_message_iter_get_arg_type(&entry) ==
+ DBUS_TYPE_STRING) {
+ const char *nameserver;
+
+ dbus_message_iter_get_basic(&entry, &nameserver);
+
+ DBG("DNS%d: %s", i, nameserver);
+
+ nameservers = g_try_renew(char *, nameservers, i + 2);
+ if (!nameservers)
+ break;
+
+ nameservers[i] = g_strdup(nameserver);
+ if (!nameservers[i]) {
+ g_strfreev(nameservers);
+ nameservers = NULL;
+ break;
+ }
+
+ nameservers[++i] = NULL;
+
+ dbus_message_iter_next(&entry);
+ }
+
+ g_strfreev(provider->transport_nameservers);
+ provider->transport_nameservers = nameservers;
}
out:
@@ -2985,6 +3022,17 @@ const char *vpn_provider_get_string(struct vpn_provider *provider,
return setting->value;
}
+char **vpn_provider_get_string_list(struct vpn_provider *provider,
+ const char *key)
+{
+ DBG("provider %p key %s", provider, key);
+
+ if (g_str_equal(key, "TransportNameservers"))
+ return provider->transport_nameservers;
+
+ return NULL;
+}
+
int vpn_provider_set_boolean(struct vpn_provider *provider, const char *key,
bool value, bool force_change)
{
diff --git a/vpn/vpn-provider.h b/vpn/vpn-provider.h
index a7ec5109..6bd64205 100644
--- a/vpn/vpn-provider.h
+++ b/vpn/vpn-provider.h
@@ -87,6 +87,8 @@ int vpn_provider_set_string_hide_value(struct vpn_provider *provider,
const char *key, const char *value);
const char *vpn_provider_get_string(struct vpn_provider *provider,
const char *key);
+char **vpn_provider_get_string_list(struct vpn_provider *provider,
+ const char *key);
bool vpn_provider_get_string_immutable(struct vpn_provider *provider,
const char *key);
int vpn_provider_set_boolean(struct vpn_provider *provider, const char *key,
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 43/45] wireguard: Add option for using transport nameservers for DNS reresolve
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (41 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 42/45] vpn-provider: Add support for set/get "TransportNameservers" Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 44/45] wireguard: Fix FQDN by using the resolved IP as the gateway Jussi Laakkonen
` (4 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
If the "WireGuard.ReresolveUseTransportDNS" is defined add the
nameservers of the transport to be used for DNS reresolve queries. This
affects only the GResolv queries done after the intial connection to
reresolve the endpoint address.
---
vpn/plugins/wireguard.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index a2d92ec6..2787aa7e 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -53,6 +53,7 @@
#define DNS_RERESOLVE_TIMEOUT 20
#define DNS_RERESOLVE_ERROR_LIMIT 5
+#define DNS_DEFAULT_PORT 53
#define ROUTE_SETUP_TIMEOUT 200 // ms
#define ARRAY_SIZE(a) (sizeof(a)/sizeof(a[0]))
@@ -90,7 +91,8 @@ struct {
{"WireGuard.PublicKey", true},
{"WireGuard.AllowedIPs", true},
{"WireGuard.EndpointPort", true},
- {"WireGuard.PersistentKeepalive", true}
+ {"WireGuard.PersistentKeepalive", true},
+ {"WireGuard.ReresolveUseTransportDNS", false}
};
static struct wireguard_info *create_private_data(struct vpn_provider *provider)
@@ -532,7 +534,7 @@ static void resolve_endpoint_cb(GResolvResultStatus status,
case G_RESOLV_RESULT_STATUS_NO_ANSWER:
case G_RESOLV_RESULT_STATUS_NO_RESPONSE:
case G_RESOLV_RESULT_STATUS_SERVER_FAILURE:
- DBG("retry DNS reresolve");
+ DBG("retry DNS reresolve, status %d", status);
if (info->provider)
vpn_provider_add_error(info->provider,
VPN_PROVIDER_ERROR_CONNECT_FAILED);
@@ -544,7 +546,7 @@ static void resolve_endpoint_cb(GResolvResultStatus status,
case G_RESOLV_RESULT_STATUS_FORMAT_ERROR:
case G_RESOLV_RESULT_STATUS_NOT_IMPLEMENTED:
case G_RESOLV_RESULT_STATUS_REFUSED:
- DBG("stop DNS reresolve, error %d", status);
+ DBG("stop DNS reresolve, status %d", status);
if (err && info->provider)
vpn_provider_add_error(info->provider,
VPN_PROVIDER_ERROR_CONNECT_FAILED);
@@ -598,7 +600,10 @@ static int disconnect(struct vpn_provider *provider, int error);
static gboolean wg_dns_reresolve_cb(gpointer user_data)
{
struct wireguard_info *info = user_data;
+ char **nameservers;
+ bool option;
int err;
+ int i;
DBG("");
@@ -617,6 +622,18 @@ static gboolean wg_dns_reresolve_cb(gpointer user_data)
DBG("endpoint_fqdn %s", info->endpoint_fqdn);
+ option = vpn_provider_get_boolean(info->provider,
+ "WireGuard.ReresolveUseTransportDNS",
+ false);
+ if (option) {
+ nameservers = vpn_provider_get_string_list(info->provider,
+ "TransportNameservers");
+ for (i = 0; nameservers && nameservers[i]; i++)
+ vpn_util_resolv_add_nameserver(info->resolv,
+ nameservers[i],
+ DNS_DEFAULT_PORT, 0);
+ }
+
info->resolv_id = vpn_util_resolve_hostname(info->resolv,
info->endpoint_fqdn,
resolve_endpoint_cb, info);
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 44/45] wireguard: Fix FQDN by using the resolved IP as the gateway
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (42 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 43/45] wireguard: Add option for using transport nameservers for DNS reresolve Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-11 14:27 ` [PATCH 45/45] provider: Add the VPN nameserver routes when connected Jussi Laakkonen
` (3 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
This fixes the hostname use with WireGuard by using the resolved IP as
the gateway for the ipaddress. This address will be sent to connmand for
setting up the routes and DNS will be otherwise broken if the FQDN is
set "as is" as the gateway.
By returning also the resolved IP in string format in the initial
resolve, and setting it as the gateway for the address for the interface
in in the ipaddress.c the FQDN using WireGuard services can be used.
---
vpn/plugins/wireguard.c | 80 +++++++++++++++++++++++++----------------
1 file changed, 50 insertions(+), 30 deletions(-)
diff --git a/vpn/plugins/wireguard.c b/vpn/plugins/wireguard.c
index 2787aa7e..98a9c71a 100644
--- a/vpn/plugins/wireguard.c
+++ b/vpn/plugins/wireguard.c
@@ -249,10 +249,35 @@ static int get_endpoint_addr(const char *host, const char *port, int flags,
return 0;
}
+static const char *endpoint_to_str(struct wg_peer *peer, char *buf,
+ socklen_t len)
+{
+ struct sockaddr_u *addr;
+ int family;
+
+ addr = (struct sockaddr_u *)&peer->endpoint.addr;
+ family = peer->endpoint.addr.sa_family;
+
+ switch (family) {
+ case AF_INET:
+ return inet_ntop(family, &addr->sin.sin_addr, buf, len);
+ case AF_INET6:
+ return inet_ntop(family, &addr->sin6.sin6_addr, buf, len);
+ default:
+ break;
+ }
+
+ return NULL;
+}
+
static int parse_endpoint_hostname(const char *host, const char *port,
- struct sockaddr_u *addr)
+ struct wg_peer *peer,
+ char **gateway_resolved)
{
+ struct sockaddr_u *addr;
char **tokens;
+ const char *gw = NULL;
+ char buf[INET6_ADDRSTRLEN] = { 0 };
unsigned int len;
int err;
@@ -271,9 +296,21 @@ static int parse_endpoint_hostname(const char *host, const char *port,
DBG("using host %s", tokens[0]);
+ addr = (struct sockaddr_u *)&peer->endpoint.addr;
+
err = get_endpoint_addr(tokens[0], port, 0, addr);
- if (!err)
+ if (!err) {
+ /*
+ * In case the endpoint is an host address use the resolved
+ * IP address as gateway for DNS over WireGuard to work.
+ */
+ if (connman_inet_check_ipaddress(tokens[0]) <= 0)
+ gw = endpoint_to_str(peer, buf, INET6_ADDRSTRLEN);
+
DBG("success");
+ }
+
+ *gateway_resolved = gw ? g_strdup(gw) : g_strdup(tokens[0]);
g_strfreev(tokens);
@@ -648,27 +685,6 @@ static gboolean wg_dns_reresolve_cb(gpointer user_data)
return G_SOURCE_REMOVE;
}
-static const char *endpoint_to_str(struct wg_peer *peer, char *buf,
- socklen_t len)
-{
- struct sockaddr_u *addr;
- int family;
-
- addr = (struct sockaddr_u *)&peer->endpoint.addr;
- family = peer->endpoint.addr.sa_family;
-
- switch (family) {
- case AF_INET:
- return inet_ntop(family, &addr->sin.sin_addr, buf, len);
- case AF_INET6:
- return inet_ntop(family, &addr->sin6.sin6_addr, buf, len);
- default:
- break;
- }
-
- return NULL;
-}
-
static gboolean wg_route_setup_cb(gpointer user_data)
{
struct wireguard_info *info = user_data;
@@ -767,7 +783,9 @@ static int wg_connect(struct vpn_provider *provider,
{
struct wg_ipaddresses ipaddresses = { 0 };
struct wireguard_info *info;
- const char *option, *gateway;
+ const char *option;
+ const char *endpoint;
+ char *gateway = NULL;
char *ifname;
bool do_split_routing = true;
int err = -EINVAL;
@@ -856,21 +874,20 @@ static int wg_connect(struct vpn_provider *provider,
if (!option)
option = "51820";
- gateway = vpn_provider_get_string(provider, "Host");
+ endpoint = vpn_provider_get_string(provider, "Host");
/*
* Use the resolve timeout only with re-resolve. Here the network
* is setup as the transport is used. In succeeding attempts resolving
* is needed as it is done over potentially misconfigured WireGuard
* connection that may end up blocking vpnd with getaddrinfo().
*/
- err = parse_endpoint_hostname(gateway, option,
- (struct sockaddr_u *)&info->peer.endpoint.addr);
+ err = parse_endpoint_hostname(endpoint, option, &info->peer, &gateway);
if (err) {
- DBG("Failed to parse endpoint %s:%s", gateway, option);
+ DBG("Failed to parse endpoint %s:%s", endpoint, option);
goto error;
}
- info->endpoint_fqdn = g_strdup(gateway);
+ info->endpoint_fqdn = g_strdup(endpoint);
info->port = g_strdup(option);
option = vpn_provider_get_string(provider, "WireGuard.Address");
@@ -878,11 +895,14 @@ static int wg_connect(struct vpn_provider *provider,
DBG("Missing WireGuard.Address configuration");
goto error;
}
+
err = parse_addresses(option, gateway, &ipaddresses);
if (err) {
- DBG("Failed to parse addresses %s gateway %s", option, gateway);
+ DBG("Failed to parse addresses %s endpoint %s gateway %s",
+ option, endpoint, gateway);
goto error;
}
+ g_free(gateway);
ifname = get_ifname();
if (!ifname) {
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 45/45] provider: Add the VPN nameserver routes when connected
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (43 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 44/45] wireguard: Fix FQDN by using the resolved IP as the gateway Jussi Laakkonen
@ 2025-07-11 14:27 ` Jussi Laakkonen
2025-07-18 16:52 ` [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (2 subsequent siblings)
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-11 14:27 UTC (permalink / raw)
To: connman
Setup the routes to nameservers when VPN has reached ready state, i.e.,
connected.
---
src/provider.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/provider.c b/src/provider.c
index e790ccea..613504ff 100644
--- a/src/provider.c
+++ b/src/provider.c
@@ -323,6 +323,10 @@ static int set_connected(struct connman_provider *provider,
provider->driver->set_routes(provider,
CONNMAN_PROVIDER_ROUTE_ALL);
+ __connman_service_nameserver_add_routes(provider->vpn_service,
+ provider->driver->get_property(
+ provider, "HostIP"));
+
} else {
if (ipconfig_ipv4) {
provider_indicate_state(provider,
--
2.39.5
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 00/46] VPN association state, dual IP support and WG fixes
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (44 preceding siblings ...)
2025-07-11 14:27 ` [PATCH 45/45] provider: Add the VPN nameserver routes when connected Jussi Laakkonen
@ 2025-07-18 16:52 ` Jussi Laakkonen
2025-07-28 14:54 ` Denis Kenzior
2025-08-04 15:10 ` patchwork-bot+connman
47 siblings, 0 replies; 69+ messages in thread
From: Jussi Laakkonen @ 2025-07-18 16:52 UTC (permalink / raw)
To: connman
Hello all
On 7/11/25 17:26, Jussi Laakkonen wrote:
> This patch set (1) adds the association state also for the VPNs, (2) implements
> dual IP support for VPNs and (3) contains fixes for Wireguard address,
> especially for FQDN handling. This is a combination of all three improvements
> for the sake of testing them together.
>
> (1): association state for VPNs
> The association state is to indicate that the VPN is waiting for VPN agent to
> provide input given by user. In this state service.c must not do connect
> timeout checks as the timers for both differ in length, default being 120s for
> connect timeout and 300s for VPN agent dialog timeout.
>
> In order to facilitate this change the association state had to be implemented
> also for VPNs. It is common state for services and like with services the
> association state for VPNs preceeds the configuration state (on VPN side
> connect state). Both vpn.c plugins on connmand and vpnd side require changes
> to accommodate this state. When the VPN agent succeeds in getting the input
> from the user the state transitions from association to connect (configuration)
> state and, thus, requires no specific changes to VPN plugins.
>
> On connmand side the association state is the initial state when VPN is getting
> connected and the state needs to be accounted as a connecting state in
> plugins/vpn.c to not to lose transport ident for it and in provider.c as a
> pre-configuration state to not to start the connect timeout for the VPN before
> the VPN is in configuration state. The reason for the latter is that the
> connect timeout should be exact and start from the point when
> connect/configuration state is entered.
>
> On vpnd side association state is, like on connmand side, the initial state for
> the VPN getting connected. After the VPN agent succeeds getting the information
> from the user (credentials) the state transitions to connect (configuratioin).
> There may be a possibility for a VPN plugin to run without VPN agent and thus
> in these cases it is ensured that the vpn/plugins/vpn.c:vpn_notify() does
> the state transition in such cases. It is allowed go back to association state
> from connect state but not from other states.
>
> (2): dual IP support for VPNs
>
> Dual IP support for VPNs is implemented by adding an family extension that
> simply uses a boolean array of 2. With this IPv4 and IPv6 can be both defined
> on a VPN such as WireGuard and provider.c will setup the addresses correctly in
> connmand.
>
> (3): WireGuard fixes
>
> This improves the WireGuard plugin and adds better error case support for the
> vpn/plugins/vpn.c. This allows also to propagate the errors upward and with
> other changes, allows the shutdown to follow the same process as the other
> VPNs. Also fix the PrefixLength use in the WireGuard plugin by tokenizing the
> host before getaddrinfo() check. One of the key fixes here is to make FQDN
> work with WireGuard. Also a new option is added for using the transport
> nameservers with WireGuard reresolve queries, which by default is off (false).
>
> First, the basic saving of the WireGuard configuration is done similarly to
> other plugins, as well as to what wg-quick is utilizing.
>
> Second, the handling of errors is improved within the plugin and vpn.c as well.
> This will make it possible to pass the errors upwards from the plugin In
> addition to this there is a limit for reresolve errors (5 by default) after
> which WireGuard plugin dies in case the configuration is wrong, or network is
> broken.
>
> Third, the use of getaddrinfo() will block with invalid configuration when
> doing the reresolve for the endpoint. This is now replaced with GResolv by
> adding a wrapper for it in vpn-util.c so it can be used within VPN plugins as
> well. This avoids the blocking of the non-existent address resolve that made
> vpnd unresponsive for the time being, for example, disconnects did not work.
>
> Fourth, the shutdown is now simulated in a same way other daemon utilizing VPNs
> do, by calling the vpn_died() with a slight delay. This makes daemonless VPNs
> work in the same way as the rest of the plugins to do the same cleanup steps.
>
> Fifth, the host given in the configuration as an IP-address should contain
> CIDR notation but as getaddrinfo() uses inet_pton(), which is relying on the
> address to not to have the notation, the host is tokenized first for this use.
>
> Sixth, there is an option added, "WireGuard.ReresolveUseTransportDNS" that can
> be set to boolean values, "true" indicating that the nameservers of the
> transport are used for the DNS reresolve queries. This may become useful in
> cases where user cannot affect their network setup outside their devices. By
> default this option is set off, and is saved among other options.
>
> Seventh, the FQDN server use is fixed by using the resolved IP of the server as
> the gateway. The reason this broke networking was that the FQDN name was sent
> to connmand "as is" and it was used as gateway, which could not be resolved
> when routing packets.
>
Forgot to add:
Tested by: Christian Hewitt & LibreELEC community
BR,
Jussi
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 00/46] VPN association state, dual IP support and WG fixes
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (45 preceding siblings ...)
2025-07-18 16:52 ` [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
@ 2025-07-28 14:54 ` Denis Kenzior
2025-08-04 15:10 ` patchwork-bot+connman
47 siblings, 0 replies; 69+ messages in thread
From: Denis Kenzior @ 2025-07-28 14:54 UTC (permalink / raw)
To: Jussi Laakkonen, connman
Hi Jussi,
On 7/11/25 9:26 AM, Jussi Laakkonen wrote:
> This patch set (1) adds the association state also for the VPNs, (2) implements
> dual IP support for VPNs and (3) contains fixes for Wireguard address,
> especially for FQDN handling. This is a combination of all three improvements
> for the sake of testing them together.
>
Now that connman 1.45 is out, I'll start reviewing these. It'll take a bit
since I have limited time these days FYI. Most likely I will cherry pick what I
can on the first round and then request additional changes.
Regards,
-Denis
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 00/46] VPN association state, dual IP support and WG fixes
2025-07-11 14:26 [PATCH 00/46] VPN association state, dual IP support and WG fixes Jussi Laakkonen
` (46 preceding siblings ...)
2025-07-28 14:54 ` Denis Kenzior
@ 2025-08-04 15:10 ` patchwork-bot+connman
47 siblings, 0 replies; 69+ messages in thread
From: patchwork-bot+connman @ 2025-08-04 15:10 UTC (permalink / raw)
To: Jussi Laakkonen; +Cc: connman
Hello:
This series was applied to connman.git (master)
by Denis Kenzior <denkenz@gmail.com>:
On Fri, 11 Jul 2025 17:26:51 +0300 you wrote:
> This patch set (1) adds the association state also for the VPNs, (2) implements
> dual IP support for VPNs and (3) contains fixes for Wireguard address,
> especially for FQDN handling. This is a combination of all three improvements
> for the sake of testing them together.
>
> (1): association state for VPNs
> The association state is to indicate that the VPN is waiting for VPN agent to
> provide input given by user. In this state service.c must not do connect
> timeout checks as the timers for both differ in length, default being 120s for
> connect timeout and 300s for VPN agent dialog timeout.
>
> [...]
Here is the summary with links:
- [01/45] agent: Cancel agent request on NoReply D-Bus error
https://git.kernel.org/pub/scm/network/connman/connman.git/?id=01c10f2ce960
- [02/45] vpn-provider: Use association state for VPN agent input wait
(no matching commit)
- [03/45] vpn: Add association state before connect state
(no matching commit)
- [04/45] vpn-agent: Do connect state transition after input dialog check
(no matching commit)
- [05/45] service: Explicit VPN connect timeout, ignore in VPN agent wait
(no matching commit)
- [06/45] provider: Handle VPN configuration and association states
(no matching commit)
- [07/45] vpn: Add support for association state, add state getter
(no matching commit)
- [08/45] vpn: Check if connecting when setting state or disconnecting
(no matching commit)
- [09/45] vpn: Add VPN agent use callback for plugins
(no matching commit)
- [10/45] vpn-provider: Transition to CONNECT state with agentless VPNs
(no matching commit)
- [11/45] doc: Update VPN documentation for association state
(no matching commit)
- [12/45] wireguard: Add saving of provider properties
(no matching commit)
- [13/45] wireguard: Use positive errors for VPN provider connect_cb
(no matching commit)
- [14/45] vpn: Fix VPN_FLAG_NO_DAEMON use in error cases
(no matching commit)
- [15/45] wireguard: Handle disconnect, error and network errors better
(no matching commit)
- [16/45] gresolv: Add generic error for GResolv struct with getter
(no matching commit)
- [17/45] vpn-util: Add wrappers for GResolv hostname lookup use
(no matching commit)
- [18/45] wireguard: Use GResolv for DNS reresolve to avoid blocking
(no matching commit)
- [19/45] vpn: Drop state changes from update_provider_state()
(no matching commit)
- [20/45] wireguard: Fix shutdown, ensure one exit and set no agent is used
(no matching commit)
- [21/45] vpn: Check if disconnect is implemented before calling in stop_vpn()
(no matching commit)
- [22/45] wireguard: Tokenize host for getaddrinfo()
(no matching commit)
- [23/45] util: Add address family set/get/reset helpers
(no matching commit)
- [24/45] vpn-provider: Add support for dual-IP VPNs
(no matching commit)
- [25/45] provider: Add support for dual-IP VPNs
(no matching commit)
- [26/45] vpn: Add support for dual-IP VPNs
(no matching commit)
- [27/45] wireguard: Support both IPv4 and IPv6 address
(no matching commit)
- [28/45] inet: Expose __connman_inet_is_any_addr() for plugins to use
(no matching commit)
- [29/45] wireguard: Set split routing based on AllowedIPs
(no matching commit)
- [30/45] Revert "vpn: Remove unused __vpn_provider_check_routes"
(no matching commit)
- [31/45] vpn-provider: Allow to add complete routes and to remove routes
(no matching commit)
- [32/45] wireguard: Add routes for other than any addresses
(no matching commit)
- [33/45] wireguard: Fix string list parsing and IP tunneling
(no matching commit)
- [34/45] wireguard: Treat initial connect failure as unreachable host
(no matching commit)
- [35/45] service: handle also EALREADY in service_connect()
(no matching commit)
- [36/45] vpn-provider: Make daemonless VPNs to connect when connmand is online
(no matching commit)
- [37/45] vpn: Implement getter for the flags set by the VPN
(no matching commit)
- [38/45] vpn-provider: Delay connect of daemonless VPNs until connmand is online
(no matching commit)
- [39/45] wireguard: Rework hostname resolve, split code and do not resolve IP
(no matching commit)
- [40/45] vpn-util: Add wrapper for adding a namerver for GResolv
(no matching commit)
- [41/45] service: Send the DNS servers of VPN's transport when VPN is ready
(no matching commit)
- [42/45] vpn-provider: Add support for set/get "TransportNameservers"
(no matching commit)
- [43/45] wireguard: Add option for using transport nameservers for DNS reresolve
(no matching commit)
- [44/45] wireguard: Fix FQDN by using the resolved IP as the gateway
(no matching commit)
- [45/45] provider: Add the VPN nameserver routes when connected
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 69+ messages in thread