diff mbox series

Message ID CAMieACadCW0Fw60Hpf9mOT17iAgSVkZRYa_6QjGyAy62y0USPA@mail.gmail.com (mailing list archive)
State New
Headers show
Series | expand

Commit Message

Scott Valentine Jan. 27, 2025, 11:41 p.m. UTC
While testing an upgrade from v1.33 to v1.43, we experienced an issue
where some of our devices were failing to establish internet
connectivity, while the older version of connman had no problems.

We have identified the source of the issue as a buggy proxy
configuration from the ISP provided router. Specifically, these are
Hawaiian Telecom provided Zyxel wireless routers. The router DHCP
server supplies a PAC URL option with essentially an empty value of
[0x0a, 0x0d, 0x00], as well as a domain name option of "home".

This ultimately results in a call to __connman_wpad_start() with an
invalid configuration that should not succeed (I think?), but does
(due to the hostname also being set by the DHCP server), even though
the PAC URL is null.

WPAD then calls __connman_service_set_proxy_autoconfig() which results
in the following broken configuration for the service proxy:
            dict entry(
               string "Proxy"
               variant                   array [
                     dict entry(
                        string "URL"
                        variant                            string
"http://wpad.home/wpad.dat"
                     )
                     dict entry(
                        string "Method"
                        variant                            string "auto"
                     )
                  ]
            )

I believe the correct fix for this could be to check the PAC URL
either before starting WPAD, or within __connman_wpad_start(), the
latter requiring a getter for the service->pac property. N.B. I don't
really know much of anything about WPAD or PAC.

For now, I can work around the issue with a small patch to force the
proxy method to "direct" in service.c: check_proxy_setup, but I'm not
super confident that this is the best approach:

                (service->proxy_config != CONNMAN_SERVICE_PROXY_METHOD_AUTO ||


Mahalo!
-Scott V.
diff mbox series

Patch

diff -uNrp old/src/service.c new/src/service.c
--- old/src/service.c   2024-09-01 23:34:15.000000000 -1000
+++ new/src/service.c   2025-01-27 13:11:52.713053125 -1000
@@ -1731,8 +1731,18 @@  static bool check_proxy_setup(struct con
         * to AUTO with an empty URL.
         */

-       if (service->proxy != CONNMAN_SERVICE_PROXY_METHOD_UNKNOWN)
+       DBG("proxy %d, config %d, pac %s", service->proxy,
service->proxy_config,
+                       service->pac);
+
+       if (service->proxy != CONNMAN_SERVICE_PROXY_METHOD_UNKNOWN) {
+               if ((service->proxy == CONNMAN_SERVICE_PROXY_METHOD_AUTO) &&
+                               (service->pac == NULL)) {
+                       DBG("OVERRIDING PROXY METHOD");
+                       connman_service_set_proxy_method(service,
+                               CONNMAN_SERVICE_PROXY_METHOD_DIRECT);
+                }
                return true;
+       }

        if (service->proxy_config != CONNMAN_SERVICE_PROXY_METHOD_UNKNOWN &&