feat(adblock): implement device-based enforcement and migration scripts - #1875
Open
m-dilorenzi wants to merge 1 commit into
Open
feat(adblock): implement device-based enforcement and migration scripts#1875m-dilorenzi wants to merge 1 commit into
m-dilorenzi wants to merge 1 commit into
Conversation
gsanchietti
approved these changes
Aug 7, 2026
Tbaile
requested changes
Aug 10, 2026
Comment on lines
+752
to
+757
| devices = [] | ||
| for zone in zones: | ||
| for device in utils.get_all_devices_by_zone(e_uci, zone, exclude_aliases=True): | ||
| if device not in devices: | ||
| devices.append(device) | ||
| return sorted(devices) |
Collaborator
There was a problem hiding this comment.
Next time you can use a set, so you don't need to check for presence in the list, for the number of occurrences, do not bother changing it
Comment on lines
+739
to
+747
| def dns_get_enforced_zones(e_uci): | ||
| # the user selection is stored in ns_tsdns_zones: adb_nftdevforce holds the | ||
| # devices derived from it and can't be mapped back to zones without ambiguity | ||
| zones = list(e_uci.get('adblock', 'global', 'ns_tsdns_zones', list=True, default=[])) | ||
| if zones: | ||
| return zones | ||
| # before the switch to device-based enforcement the selection was stored inside | ||
| # adb_nftdevforce: keep reading it until the migration has run | ||
| return list(e_uci.get('adblock', 'global', 'adb_nftdevforce', list=True, default=['lan'])) |
Collaborator
There was a problem hiding this comment.
After the uci default, this is not needed, right?
We can safely remove it and put this logic underneath
| $(INSTALL_BIN) ./files/adjust-banip.py $(1)/usr/libexec/ns-api/post-commit/ | ||
| $(INSTALL_BIN) ./files/configure-banip-wans.py $(1)/usr/libexec/ns-api/pre-commit/ | ||
| $(INSTALL_BIN) ./files/configure-adblock-devices.py $(1)/usr/libexec/ns-api/pre-commit/ | ||
| $(INSTALL_DIR) $(1)/usr/libexec |
Collaborator
There was a problem hiding this comment.
Move it at the top where all install dir resides, this missing could've cause issues.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes the issue where DNS queries are no longer redirected to dnsmasq after the adblock upgrade to 4.5.5.
Up to adblock 4.1.5 the local DNS enforcement was configured with firewall zone names, and adblock turned them into UCI redirect sections letting fw4 resolve each zone into its devices. Since 4.5.5 adblock writes nftables rules directly and expects the
adb_nftdevforceoption to contain device names, because its values are used as-is in theiifnamematch. Zone names kept being stored there, so the resulting rules never matched any packet and DNS traffic was not intercepted.The fix makes
adb_nftdevforcea derived value: the zones selected in the UI are stored in a dedicatedns_tsdns_zonesoption and expanded into the devices of those zones. The UI is not affected, it keeps working with zones.Changes
packages/ns-api/files/ns.threatshield:dns_edit_settingsnow saves the selected zones inns_tsdns_zonesand writes the devices they contain intoadb_nftdevforce;dns_list_settingsreads the zones back from the new option, with a fallback that keeps the settings page working until the migration has run.packages/ns-threat_shield/files/configure-adblock-devices.py(new): pre-commit hook that recomputesadb_nftdevforcewheneveradblock,firewallornetworkchange, so that adding an interface to an enforced zone does not leave its DNS traffic unfiltered.packages/ns-threat_shield/files/adblock-devices-migrate.py(new): rebuilds the configuration on systems already upgraded, which would otherwise stay broken until someone re-saves the Threat Shield DNS settings, and restarts adblock.packages/ns-threat_shield/files/36_ns-threat_shield(new): uci-defaults running the migration above, following the same pattern used for the wireguard migration.packages/ns-threat_shield/Makefile: installs the new hook, migration and uci-defaults, and raises thens-apirequirement so the migration cannot be undone by an older API still writing zone names.Closes: #1871