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

ASUSWRT-Merlin dnsmasq.postconf not allowing other script commands to execute #115

Closed
dave14305 opened this issue Mar 31, 2020 · 14 comments
Closed

Comments

@dave14305
Copy link
Contributor

The current implementation of the dnsmasq.postconf for ASUSWRT-Merlin prepends the necessary commands to an existing dnsmasq.postconf file, but includes an exit 0 which prevents further commands in the existing script from running. This is a bad design in my opinion.

If you want to ensure the nextdns commands run last and have the final say, then append the content to the existing file.

https://github.com/nextdns/nextdns/blob/master/router/merlin/setup.go#L99

@rs
Copy link
Contributor

rs commented Mar 31, 2020

The idea is that most other changes to the dnsmasq conf will conflict with this one. Do you have an example of another service that would be compatible?

@dave14305
Copy link
Contributor Author

In my case, I setup additional dnsmasq records based on nvram variables, so I must do it in a script instead of dnsmasq.conf.add. This includes local host-records, server statements to prevent non-public work-related domain names from being sent upstream, etc.

I see how it can be a conflict if someone on Merlin redirects to another service on 127.0.0.1. Maybe you can also include a pc_delete "server=127.0.0.1" "$CONFIG" at the beginning of the if true block.

@rs
Copy link
Contributor

rs commented Mar 31, 2020

Why don’t you put local host records in the /etc/hosts file?

@rs
Copy link
Contributor

rs commented Mar 31, 2020

You can also use forwarder config with nextdns to send some private domain to somewhere else.

@dave14305
Copy link
Contributor Author

Those were just examples in my own setup. I don't want to debate the merits of whether I should be using dnsmasq.postconf, but just to point out that it's considered heavy-handed in the Merlin community to block any other content from being executed. He who executes last in the script wins. I think the delete of server=127.0.0.1 would avoid conflicts with Unbound or dnscrypt-proxy.

Loading Diversion blockfiles into dnsmasq is a delicate issue if you were to force delete them when nextdns is active, but you could do it with
pc_delete "addn-hosts=/opt/share/diversion/list/" "$CONFIG"

@rs
Copy link
Contributor

rs commented Mar 31, 2020

I could append last and revert all possible alterations of the dnsmasq config that could conflict. I just feel like it will be much more brittle.

@rs
Copy link
Contributor

rs commented Apr 1, 2020

As I can't access my router lab, I can't test such a change. Would you like to give it a try @dave14305?

@dave14305
Copy link
Contributor Author

I’m back again, running the nextdns CLI on ASUSWRT-Merlin-LTS (John’s fork) and it works well except for the dnsmasq.postconf hijacking (hope it’s not too strong a word). A specific concrete case now is that later in the dnsmasq.postconf is the Diversion command which will enable dnsmasq logging, which becomes important when troubleshooting issues with upstream providers.

So if you have a proposed build to try, I’m willing to test it out.

I think it might also be time to cut back on the “Received signal” messages. I think you solved that issue well enough.

@rs
Copy link
Contributor

rs commented Apr 19, 2020

I will try to find a solution to that ASAP. I'm just curious, why running diversion + nextdns? Isn't it a little redundant?

@dave14305
Copy link
Contributor Author

It’s valuable even with ad-blocking disabled because it manages dnsmasq logging and log rotation.

@rs
Copy link
Contributor

rs commented Apr 19, 2020

Honestly, with caching on in the cli, you could just put port 0 in dnsmasq config and get rid of it for DNS. That is what the setup-router does on all other platforms, but merlin community seems to be very attached to dnsmasq for some reason I can not gasp :)

@dave14305
Copy link
Contributor Author

It’s often pondered by the users tinkering with Unbound, but in the end, dnsmasq is so ingrained in the firmware that trying to neuter it is daunting. Would nextdns handle resolution of local hostnames found via discovery? And deal with VPN clients running on the router? It gets messy.

@rs
Copy link
Contributor

rs commented Apr 19, 2020

Yes, I’m thinking about adding this capability.

@preacher65
Copy link

Hi, just wondered if this is still being considered?
Do I break anything by commenting out the exit 0?

@dave14305 dave14305 closed this as not planned Won't fix, can't repro, duplicate, stale Feb 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants