adblock-lean
adblock-lean copied to clipboard
lynxthecat
Improving integration with the OpenWrt UCI framework
Great program.
Here are some ideas to make the program more integrated/dynamic with the rest of Openwrt. Some of these ideas may be easier to implement than others, so this is more a wishlist
1) Append option confdir to the selected dnsmasq instance for ad blocking (if such option does not already exist)
During the current setup process, the code pulls dnsmasq configuration using /etc/init.d/dnsmasq info and then parses it for things like the list of dnsmasq instances, confdir folders and names.
Here is an alternate way that likely requires much less code and would have some other benefits. Use /etc/config/dhcp itself to parse the configuration of dnsmasq. This would require that the confdir option be set explicitly for the selected dnsmasq instance for adblocking, as I don't believe Openwrt sets this option by default on the initial installation. So the setup code would parse /etc/config/dhcp for the list of dndmsasq instances, and once the user selects the instance(s) for ad blocking, the setup program would append an option with the confdir like:
uci set dhcp.@dnsmasq[0].confdir='/tmp/dnsmasq.adblock-lean.d'
uci commit
This has many possible advantages:
- The user doesn't have to run adblock-lean setup again if they change the confdir later in /etc/config/dhcp
- Fewer configuration values for adblock-lean are needed. The program now can just use DNSMASQ_INDEX to get the confdir later. It does not need to store DNSMASQ_INSTANCE or DNSMASQ_CONF_D, so two less configuration values needed.
- Eliminates a bunch of JSON parsing code from adblock-lean, as the current code has to parse through the JSON output from /etc/init.d/dnsmasq info` whereas now the dnsmasq configuration can all be simply read from the UCI configuration
- Avoids possible race condition. By having adblock-lean read the dnsmasq configuration from the Openwrt UCI system, adblock-lean could potentially download and store the blocklists in the right conf-dir even before dnsmasq is initialized
If there is a concern about appending another option to the dhcp configuration file in the Openwrt UCI Configuration System, adblock-lean is already appending an option for addnmount to enable blocklist decompression. So it would be adding 2 options to dhcp instead of just addnmount. If there is a concern about changing the default confdir folder for the selected instance of dnsmasq, the default folder is already rather arbitrary so might as well be declared explicitly
2) Allow user to select multiple dnsmasq instances for adblocking.
I have multiple dnsmasq instances running on Openwrt (think kid safe parental control vs. non kid safe). I want both instances to use the same blocklist for ad blocking. This only requires two lines of configuraiton for each instance. So the suggestion would be to have the user select one or more dnsmasq instances in the setup process, and then adblock-lean's setup program would append two options to /etc/config/dhcp for each instance. For example:
uci add_list dhcp.@dnsmasq[0].addnmount='/bin/busybox'
uci add_list dhcp.@dnsmasq[0].addnmount='/tmp/dnsmasq.adblock-lean.d'
uci add_list dhcp.@dnsmasq[1].addnmount='/bin/busybox'
uci add_list dhcp.@dnsmasq[1].addnmount='/tmp/dnsmasq.adblock-lean.d'
This allows both instances to use the same blocklist by adblock-lean
3) Use the Openwrt UCI system for adblock-lean configuration instead of a proprietary INI file.
Given that most of Openwrt's configuration uses the UCI system, it would be nice for adblock-lean to also use this format for the configuration file. This would allow users to use the standard uci commands to easily get/set/delete configuration for adblock-lean in the same way as the rest of the Openwrt configuration (and many other Openwrt packages). For example, adding a new blocklist would be just:
uci add_list adblock-lean.adblock.blocklist_urls='https://raw.githubusercontent.com/hagezi/dns-blocklists/main/wildcard/pro-onlydomains.txt'
uci commit
My guess is this is not easy to change in adblock lean, but if so, a uci configuration file could be generated in /etc/config/adblock-lean as follows:
config adblock
option whitelist_mode '0'
# support multiple lists
list blocklist_urls 'https://raw.githubusercontent.com/hagezi/dns-blocklists/main/wildcard/pro-onlydomains.txt'
list blocklist_urls 'https://raw.githubusercontent.com/hagezi/dns-blocklists/main/wildcard/tif-onlydomains.txt'
list blocklist_ipv4_urls ''
list allowlist_urls ''
list dnsmasq_blocklist_urls ''
list dnsmasq_blocklist_ipv4_urls ''
list dnsmasq_allowlist_urls ''
option local_allowlist_path '/etc/adblock-lean/allowlist'
option local_blocklist_path '/etc/adblock-lean/blocklist'
list test_domains 'google.com'. #support multiple test domains as a list
list test_domains 'microsoft.com'
list test_domains 'amazon.com'
option list_part_failed_action 'SKIP'
option max_download_retries '3'
option min_good_line_count '340000'
option min_blocklist_part_line_count '1'
option min_blocklist_ipv4_part_line_count '1'
option min_allowlist_part_line_count '1'
option max_file_part_size_KB '24000'
option max_blocklist_file_size_KB '29000'
option deduplication '1'
option use_compression '1'
option unload_blocklist_before_update 'auto'
option boot_start_delay_s '10'
option MAX_PARALLEL_JOBS 'auto'
option custom_script ''
option cron_schedule '0 5 * * *'
# support multiple dnsmasq instances for ad blocking as a list
list DNSMASQ_INDEX '0'
list DNSMASQ_INDEX '1'
Hi and thank you for feedback and suggestions. I'll address them one by one.
About setting the conf-dir: generally, my approach to this is to avoid making system configuration changes as much as possible. The vast majority of users are fine with adblock-lean using the default conf-dir, and power users who really need the conf-dir to be non-default can easily set that conf-dir by themselves by editing /etc/config/dhcp. So I can't see much benefit in adblock-lean setting the conf-dir for them.
As to parsing the json output of /etc/init.d/dnsmasq info. The reason for doing that (rather than parsing /etc/config/dhcp) is that we do not have a reliable way to tell whether the settings in /etc/config/dhcp are actually currently applied, and whether the selected dnsmasq instance is actually runnning, unless we look at the runtime info. If we do not check whether the settings are actually applied and whether the selected instance is actually running then there may be conditions when given system is messed up and we are helping to mess it up further. There may also be conditions where we incorrectly interpret /etc/config/dhcp, as options (and their meaning) in that file may change across different OpenWrt versions (for that matter, the default conf-dir changed just recently in 24.10).
The easiest way to look at the runtime info I found was by getting that info via aforementioned command and then parsing the json output and following up to parse the conf-files.
Moreover, this provides a reliable way to figure out which network interfaces are used by each instance - information which is displayed to the user when they are selecting the dnsmasq instance, and used by adblock-lean when verifying that adblocking and dns are working (for that, we need to know the network interfaces for the instance).
All that is done by get_dnsmasq_instances() here, and the whole implementation takes 80 lines, including comments and a few empty lines. So not so much code but IMO important functionality.
About this:
- The user doesn't have to run adblock-lean setup again if they change the confdir later in /etc/config/dhcp
- Fewer configuration values for adblock-lean are needed. The program now can just use DNSMASQ_INDEX to get the confdir later. It does not need to store DNSMASQ_INSTANCE or DNSMASQ_CONF_D, so two less configuration values needed.
Generally, adblock-lean does not actually need all 3 options:
DNSMASQ_INSTANCE="<instance_name>"
DNSMASQ_INDEX="<index>"
DNSMASQ_CONF_D="<conf_dir>"
As long as we know the instance name or the instance index, we can (and do) get all the other info for that instance automatically. The reason all 3 options are required is to detect conditions when the user (or an application) makes changes to dnsmasq instances in /etc/config/dhcp which could break adblocking or cause it to attach to the wrong dnsmasq instance, and make the user aware of these conditions. This does require the user to run an extra command (service adblock-lean select_dnsmasq_instance) after they make changes to dnsmasq instances but IMO this is better than silently taking the chance of adblocking on the wrong dnsmasq instance or using the wrong conf-dir. So we could (for example) just save the instance name and ignore changes to instances, as you are suggesting, but I intentionally implemented this the way it is because IMO this is a more responsible way to handle configuration of system services (such as dnsmasq).
As to making it possible to have adblocking on multiple dnsmasq instances. We've been thinking about this but this feature comes with a side-effect of increased memory use, which is a condition which our code (specifically the preset system) currently is not designed to handle. Moreover, I imagine that some users would want to have different list selection for different dnsmasq instances, and this is currently not possible without significant changes in many other parts of the code.
Maybe allowing same blocklist to be used by multiple dnsmasq instances could be an interesting intermediate feature. Perhaps we could keep the list in some other directory (rather than inside the conf-dir of a specific instance) and configure multiple dnsmasq instances to use that list. There is still the drawback of increased memory use. This may be fine for machines which have lots of RAM. So this is worth to consider.
About using the UCI system. This is on our radar and we've been discussing this occasionally. The main reason we did not go for it is that our current config system better fits adblock-lean. Specifically:
- It validates config keys and config values - UCI doesn't.
- It is able to detect things like missing double-quotes or wrong newline style (for example,
\r\nwhich is a Windows style) and make the user aware of the issue, rather than failing with a general error message like UCI would. - It implements automated config repair or migration between versions - UCI doesn't.
- The resulting config file is more compact than a UCI config file.
We could implement UCI parser and similar functionality to what we have right now, but parsing UCI is more difficult than parsing the current (simpler) config format. This is not a show-stopper but I'm not sure it's worth the effort.
As to making it possible to have adblocking on multiple dnsmasq instances. We've been thinking about this but this feature comes with a side-effect of increased memory use, which is a condition which our code (specifically the preset system) currently is not designed to handle. Moreover, I imagine that some users would want to have different list selection for different dnsmasq instances, and this is currently not possible without significant changes in many other parts of the code.
Maybe allowing same blocklist to be used by multiple dnsmasq instances could be an interesting intermediate feature. Perhaps we could keep the list in some other directory (rather than inside the conf-dir of a specific instance) and configure multiple dnsmasq instances to use that list. There is still the drawback of increased memory use. This may be fine for machines which have lots of RAM. So this is worth to consider.
The intermediate feature is how I have it configured manually. Specifically, I have two instances of dnsmasq running that both share the same blocklist confdir folder from adblock-lean. The first DNSMASQ instance uses the upstream DNS servers of my ISP (which is essentially unfiltered) and the second DNSMASQ instance points to the upstream DNS resolver provided by Cloudflare for Families (1.1.1.3 and 1.0.0.3) which has additional content filtering for children (e.g. Cloudflare resolves Google to Safesearch, no gambling, reddit, etc.)
Here is how I manually configure it:
# instance 1 uses the ISP DNS resolver
uci add_list dhcp.@dnsmasq[0].confdir='/tmp/dnsmasq.adblock-lean.d' #common confdir
uci add_list dhcp.@dnsmasq[0].addnmount='/bin/busybox' #added by adblock-lean setup
uci commit
# instance 2 uses Cloudflare for Families for additional filtering
uci add_list dhcp.@dnmsasq[1].server='1.1.1.3'
uci add_list dhcp.@dnmsasq[1].server='1.0.0.3'
uci add_list dhcp.@dnsmasq[1].confdir='/tmp/dnsmasq.adblock-lean.d' #common confdir
uci add_list dhcp.@dnsmasq[1].addnmount='/bin/busybox'
uci commit
While manual configuration works fine, the option addnmount='bin/busybox' really seems proprietary to adblock-lean, as I've not seen any other use for it. In this example, I have adblock-lean blocklist compression enabled, so the second instance also needs the addnblock=busybox option as well. So it would be nice to have adblock-lean manage that option specific to adblock-lean during setup for one or more instances (e.g. during setup, the user selects which instance(s) will use the shared blocklist created by adblock-lean, and then the setup process configures the confdir to match and the addnmount options required for each of those instances).
This isn't required, but would be nice to have in future release. Or at least a comment in the readme for those seeking multiple instances and a shared blocklist
Support for adblocking on multiple dnsmasq instances is now implemented with PR #183 (not yet merged to master).
Could you test on your system and let us know if this works for you?
To test:
- Consider creating a backup of /etc/config/dhcp and /etc/adblock-lean/config, in case you need to later revert the changes
service adblock-lean stop- Remove any custom addnmount entries from /etc/config/dhcp
service adblock-lean update -v branch=support_adblocking_on_multiple_instances(answernwhen asked whether you want to start adblock-lean after the update)service adblock-lean select_dnsmasq_instancesservice adblock-lean start
If you want to later go back to current adblock-lean release:
service adblock-lean update -v release
The code has been merged into the master branch now and the support_adblocking_on_multiple_instances branch has been removed. If you want to test the implementation, use the command
service adblock-lean update -v snapshot
Thanks so much. I tested this and it worked for me. This can be closed.
I started from scratch. I configured three dnsmasq instances for testing purposes. I then installed the adblock-lean script with
sh /tmp/abl-install.sh -v snapshot
During the setup process, the script prompted for which dnsmasq instances to use, and I was able to select both the first and second instance by entering "0 1" when prompted.
The setup script then configured /etc/dhcp appropriately and added two addnmount options to each dnsmasq instance. It did not add the addnmount option to the third dnsmasq instance, which was expected.
All good here.