[PATCH v1 0/2] Disable online check for real

[PATCH v1 0/2] Disable online check for real

[Daniel Wagner]

Страхиња reported we still do some online checks even though
EnableOnlineCheck is set to false. Let's move the
__connman_service_wispr_start() code to helper and add a check there.

Cc: Страхиња Радић <contact@strahinja.org>

Daniel Wagner (2):
  service: Move wispr start code into helper
  service: Do not trigger wispr start when EnableOnlineCheck is disabled

 src/service.c | 37 +++++++++++++++++++++----------------
 1 file changed, 21 insertions(+), 16 deletions(-)

-- 
2.32.0

[PATCH v1 1/2] service: Move wispr start code into helper

[Daniel Wagner]

We want to be able to extend the functionally around the
__connman_service_wispr_start() code. Instead having to update the
open code part for IPv4 and IPv6 handling, let's move into a small
helper.
---
 src/service.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/service.c b/src/service.c
index a5a5cfde30fd..237c88e15506 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1595,6 +1595,19 @@ bool __connman_service_index_is_default(int index)
 	return __connman_service_get_index(service) == index;
 }
 
+static void start_wispr_when_connected(struct connman_service *service)
+{
+	if (__connman_service_is_connected_state(service,
+			CONNMAN_IPCONFIG_TYPE_IPV4))
+		__connman_service_wispr_start(service,
+					CONNMAN_IPCONFIG_TYPE_IPV4);
+
+	if (__connman_service_is_connected_state(service,
+			CONNMAN_IPCONFIG_TYPE_IPV6))
+		__connman_service_wispr_start(service,
+					CONNMAN_IPCONFIG_TYPE_IPV6);
+}
+
 static void default_changed(void)
 {
 	struct connman_service *service = connman_service_get_default();
@@ -1619,15 +1632,7 @@ static void default_changed(void)
 				connman_setting_get_bool("AllowDomainnameUpdates"))
 			__connman_utsname_set_domainname(service->domainname);
 
-		if (__connman_service_is_connected_state(service,
-						CONNMAN_IPCONFIG_TYPE_IPV4))
-			__connman_service_wispr_start(service,
-						CONNMAN_IPCONFIG_TYPE_IPV4);
-
-		if (__connman_service_is_connected_state(service,
-						CONNMAN_IPCONFIG_TYPE_IPV6))
-			__connman_service_wispr_start(service,
-						CONNMAN_IPCONFIG_TYPE_IPV6);
+		start_wispr_when_connected(service);
 
 		/*
 		 * Connect VPN automatically when new default service
@@ -3726,13 +3731,7 @@ static DBusMessage *set_property(DBusConnection *conn,
 		nameserver_add_all(service, CONNMAN_IPCONFIG_TYPE_ALL);
 		dns_configuration_changed(service);
 
-		if (__connman_service_is_connected_state(service,
-						CONNMAN_IPCONFIG_TYPE_IPV4))
-			__connman_service_wispr_start(service, CONNMAN_IPCONFIG_TYPE_IPV4);
-
-		if (__connman_service_is_connected_state(service,
-						CONNMAN_IPCONFIG_TYPE_IPV6))
-			__connman_service_wispr_start(service, CONNMAN_IPCONFIG_TYPE_IPV6);
+		start_wispr_when_connected(service);
 
 		service_save(service);
 	} else if (g_str_equal(name, "Timeservers.Configuration")) {
-- 
2.32.0

[PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled

[Daniel Wagner]

Do not try to use wispr when the user has disabled the online check.

Reported by Страхиња Радић
---
 src/service.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/service.c b/src/service.c
index 237c88e15506..8097f5373250 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1597,6 +1597,12 @@ bool __connman_service_index_is_default(int index)
 
 static void start_wispr_when_connected(struct connman_service *service)
 {
+	if (!connman_setting_get_bool("EnableOnlineCheck")) {
+		connman_info("Online check disabled. "
+			"Default service remains in READY state.");
+		return;
+	}
+
 	if (__connman_service_is_connected_state(service,
 			CONNMAN_IPCONFIG_TYPE_IPV4))
 		__connman_service_wispr_start(service,
-- 
2.32.0

Re: [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled

[Страхиња Радић]

[-- Attachment #1: Type: text/plain, Size: 226 bytes --]

On 21/08/29 09:53, Daniel Wagner wrote:
> Do not try to use wispr when the user has disabled the online check.

The patch you proposed seems fine to me. The gist is to let the user control if
the online check happens or not.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled

[Daniel Wagner]

On Sun, Aug 29, 2021 at 10:08:49PM +0200, Страхиња Радић wrote:
> On 21/08/29 09:53, Daniel Wagner wrote:
> > Do not try to use wispr when the user has disabled the online check.
> 
> The patch you proposed seems fine to me. The gist is to let the user control if
> the online check happens or not.

Could you give them a quick test if they work for you? Thanks!

Re: [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled

[Страхиња Радић]

[-- Attachment #1: Type: text/plain, Size: 1235 bytes --]

On 21/08/30 08:44, Daniel Wagner wrote:
> On Sun, Aug 29, 2021 at 10:08:49PM +0200, Страхиња Радић wrote:
> > On 21/08/29 09:53, Daniel Wagner wrote:
> > > Do not try to use wispr when the user has disabled the online check.
> > 
> > The patch you proposed seems fine to me. The gist is to let the user control if
> > the online check happens or not.
> 
> Could you give them a quick test if they work for you? Thanks!

I tested the patch with tcpdump, but unfortunately, it seems the connections are
still being made, even with EnableOnlineCheck=false, Enable6to4=false and
EnableOnlineToReadyTransition=false in /etc/connman/main.conf.

I first uninstalled and pkilled any existing versions of connman, then ensured I
applied both of your patches with git apply, ran make clean && make and make
install as root. Then I ran connmand as root and inside connmanctl:

connmanctl> enable wifi
connmanctl> agent on
connmanctl> connect <my_network>
<entered passphrase>

at the same time, I used tcpdump -i wlan0 > log.txt

and in the log I got a number of lines containing ipv4.connman.net.

Then I tried to apply my patch on top of your patches and repeat the process,
but got the same result.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled

[Страхиња Радић]

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

On 21/08/30 12:34, Страхиња Радић wrote:
> I tested the patch with tcpdump, but unfortunately, it seems the connections are
> still being made, even with EnableOnlineCheck=false, Enable6to4=false and
> EnableOnlineToReadyTransition=false in /etc/connman/main.conf.

I found the culprit -- when built from source, default prefix is /usr/local, so
connman was looking for main.conf in /usr/local/etc/connman instead of
/etc/connman. When I symlinked /etc/connman/main.conf to
/usr/local/etc/connman/main.conf and repeated the test, there were no
occurrences of ipv4.connman.net in the log. I also repeated the test using only
your two patches, without my patch, and got the same result.

Your two patches combined look ok.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v1 2/2] service: Do not trigger wispr start when EnableOnlineCheck is disabled

[Daniel Wagner]

On Mon, Aug 30, 2021 at 01:02:19PM +0200, Страхиња Радић wrote:
> On 21/08/30 12:34, Страхиња Радић wrote:
> > I tested the patch with tcpdump, but unfortunately, it seems the connections are
> > still being made, even with EnableOnlineCheck=false, Enable6to4=false and
> > EnableOnlineToReadyTransition=false in /etc/connman/main.conf.
> 
> I found the culprit -- when built from source, default prefix is /usr/local, so
> connman was looking for main.conf in /usr/local/etc/connman instead of
> /etc/connman. When I symlinked /etc/connman/main.conf to
> /usr/local/etc/connman/main.conf and repeated the test, there were no
> occurrences of ipv4.connman.net in the log. I also repeated the test using only
> your two patches, without my patch, and got the same result.
> 
> Your two patches combined look ok.

Perfect! Thanks for the testing. In this case I'll queue them up this
evening.

Daniel

Re: [PATCH v1 0/2] Disable online check for real

[Daniel Wagner]

On Sun, Aug 29, 2021 at 09:53:19PM +0200, Daniel Wagner wrote:
> Страхиња reported we still do some online checks even though
> EnableOnlineCheck is set to false. Let's move the
> __connman_service_wispr_start() code to helper and add a check there.
> 
> Cc: Страхиња Радић <contact@strahinja.org>

Patches applied.