Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

https-dns-proxy: Multiple issues with running configuration and backup configuration #23469

Closed
Shine- opened this issue Feb 17, 2024 · 5 comments

Comments

@Shine-
Copy link

Shine- commented Feb 17, 2024

Maintainer: @stangri
Environment: at least 23.05.2, likely all targets, tested with multiple ath79 and mt7621 devices
Package version: 2023-10-25-1 and 2023-10-25-5

Description:

I started playing with this package recently out of interest, and noticed that it's not quite working properly. It somehow fails to configure the dnsmasq settings correctly. The first config after installation is always fine. The problems start e.g. after reboot (-> dnsmasq settings lost) or after multiple changes e.g. using LuCI (-> backup config overwritten with running config).

Let me explain in more detail.

All these examples are done off a fresh, vanilla 23.05.2 or recent 23.05-SNAPSHOT setup, no special config, only curl and https-dns-proxy (including its LuCI package) installed on top.

Issue 1: dnsmasq config lost after reboot

This one is easy to reproduce. Install on 23.05.2 using opkg or LuCI, don't change any settings, verify that the config got set up properly, e.g.:

# uci show dhcp
[...]
dhcp.@dnsmasq[0].doh_backup_noresolv='-1'
dhcp.@dnsmasq[0].noresolv='1'
dhcp.@dnsmasq[0].doh_backup_server=''
dhcp.@dnsmasq[0].server='/mask.icloud.com/' '/mask-h2.icloud.com/' '/use-application-dns.net/' '127.0.0.1#5053' '127.0.0.1#5054'
dhcp.@dnsmasq[0].doh_server='127.0.0.1#5053' '127.0.0.1#5054'
[...]

Reboot the device. After reboot, check whether https-dns-proxy is running (e.g. via GUI or service https-dns-proxy status), then check config again - it's gone, ie. back to pre-package-installation config:

# service https-dns-proxy status
running
# uci show dhcp
[...] <--(none of the above config properties exists)

Restart service and check again, now it's there:

# service https-dns-proxy restart
Starting https-dns-proxy 2023-10-25-5 instances ✓✓
Updating dnsmasq config ✓
Restarting dnsmasq on_config_update ✓
# uci show dhcp
[...]
dhcp.@dnsmasq[0].doh_backup_noresolv='-1'
dhcp.@dnsmasq[0].noresolv='1'
dhcp.@dnsmasq[0].doh_backup_server=''
dhcp.@dnsmasq[0].server='/mask.icloud.com/' '/mask-h2.icloud.com/' '/use-application-dns.net/' '127.0.0.1#5053' '127.0.0.1#5054'
dhcp.@dnsmasq[0].doh_server='127.0.0.1#5053' '127.0.0.1#5054'
[...]

Issue 2: dnsmasq backup configuration contains running configuration after some config changes

This takes some configuration changes to reproduce, but it will happen eventually. For example, try adding a third DoH provider using LuCI, and change its settings a couple of times, press Apply & Save after each change. It won't take long, maybe 2 or 3 changes, and you'll see something like the following:

# uci show dhcp
[...]
dhcp.@dnsmasq[0].doh_backup_noresolv='-1'
dhcp.@dnsmasq[0].noresolv='1'
dhcp.@dnsmasq[0].doh_backup_server='' '/mask.icloud.com/' '/mask-h2.icloud.com/' '/use-application-dns.net/' '127.0.0.1#5053' '127.0.0.1#5054'
dhcp.@dnsmasq[0].server='/mask.icloud.com/' '/mask-h2.icloud.com/' '/use-application-dns.net/' '127.0.0.1#5053' '127.0.0.1#5054' '127.0.0.1#5055'
dhcp.@dnsmasq[0].doh_server='127.0.0.1#5053' '127.0.0.1#5054' '127.0.0.1#5055'
[...]

Note how the .doh_backup_server attribute (which should be just '') will be appended with the current running configuration. This also happens if the original .server attribute already contained one or more forwarders.

Obviously, this will cause an invalid configuration to be written to dnsmasq's .server attribute at service stop:

# service https-dns-proxy stop
Stopping https-dns-proxy 2023-10-25-5 ✓
# uci show dhcp
[...]
dhcp.@dnsmasq[0].server='/mask.icloud.com/' '/mask-h2.icloud.com/' '/use-application-dns.net/' '127.0.0.1#5053' '127.0.0.1#5054'
[...]

(the above attribute should be empty/deleted after stopping https-dns-proxy)

Issue 3: The package will mess with dnsmasq saved config, even without any settings change, at every service start/stop

Since most devices running OpenWrt are flash based, this will add to the flash wear, ie. reduce lifetime, which is bad-bad-bad. The package should not, ever, alter the saved dnsmasq config. It should do its job using the running config, which is kept in RAM (/tmp/run), not in flash.

Also, one package (https-dns-proxy) should never add new attributes into another package's config (dnsmasq), e.g. dhcp.@dnsmasq[0].doh_[...]. This is bad coding for one. And it's prone to settings loss, when the original package (ie. dnsmasq) or for example LuCI cleans invalid settings off the config.

https-dns-proxy should save its non-volatile settings only to its own UCI config. All volatile settings (like whatever the package is doing to the dnsmasq running config) should only be altered in RAM.

Issue 4 (feature request): Make the DoT intercept configurable via LuCI

It's strange that intercepting Mozilla or iCloud DoH is configurable via GUI, but intercepting DoT (Port 853) is only configurable via CLI. Also, why are these interception options implemented using a dropdown box control, instead of a simple check box? Looks a bit awkward, imo.

@stangri
Copy link
Member

stangri commented Feb 18, 2024

  1. Can't reproduce.
  2. Please include exact steps to reproduce.
  3. Incorrect, you'd benefit from checking out the README.
  4. I don't feel it warrants crowding WebUI.

@Shine-
Copy link
Author

Shine- commented Feb 18, 2024

  1. Can reproduce every time.
  2. Steps already given, can also be reproduced quite reliably.
  3. It is true, you can check file time stamps in the overlay file system - /etc/config/dhcp does get written at every service start/stop (when the dnsmasq_config_update option is set, which is set by default); the rest is also true.
  4. Fair enough. Just a suggestion, but it's your package after all.

I was just trying to help by opening this issue, you can use it or ignore it. I'm leaving it at this.
Cheers and bye.

@ghost
Copy link

ghost commented Feb 18, 2024

I have the exact same issue. After rebooting the router https-dns-proxy does not work. After service restart https-dns-proxy works again.
But I have suspicions that the problem is in the OpenWrt itself.
"luci-app-wifischedule" also not fully functional.
These problems appeared after 23.05.2 installation.
Tp-Link Archer c6u (mt7621)
So the problem actually exists.

@stangri
Copy link
Member

stangri commented Feb 18, 2024

I have the exact same issue. After rebooting the router https-dns-proxy does not work.

Not the same issue, @Shine- claims that dnsmasq servers and noresolv settings are wiped on reboot. You may want to create a separate issue if https-dns-proxy doesn't start on boot, as I will need some logs to try to address that.

@stangri
Copy link
Member

stangri commented Feb 18, 2024

Given that I can't act on This takes some configuration changes to reproduce, but it will happen eventually., closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants