From 51a61623d47d68ed03c820ab2a35b567fb509d20 Mon Sep 17 00:00:00 2001 From: Giacomo Sanchietti Date: Wed, 13 May 2026 11:47:23 +0200 Subject: [PATCH] fix(adblock): resolve race condition on rapid DNS domain changes Store whitelist/blacklist in UCI instead of direct file writes. When multiple rapid API calls add/edit/delete domains, the API now only stages changes to UCI (no restart). The init.d script calls f_write_local_lists() during start_service() and reload_service() to write files from UCI. procd's existing reload trigger (PROCD_RELOAD_DELAY=5s) debounces rapid UCI commits into a single reload_service call. The reload action (not restart) prevents f_rmdns (file clearing) that was causing "running with 0 blocked domains" broken state during concurrent restarts. Changes: - ns.threatshield: dns_get_local_list() reads from UCI; dns_write_local_list() writes to UCI + save (no commit/restart) - adblock.init: f_write_local_lists() writes UCI lists to physical files during service start/reload - 99_adblock_migrate_lists.sh: one-time migration of existing files to UCI - Makefile: install migration script to /etc/uci-defaults/ Closes #1572 Assisted-by: Copilot:claude-haiku-4.5 --- packages/adblock/Makefile | 3 ++ .../adblock/files/99_adblock_migrate_lists.sh | 24 +++++++++ packages/adblock/files/adblock.init | 23 +++++++++ packages/ns-api/files/ns.threatshield | 50 +++++++++++++------ 4 files changed, 85 insertions(+), 15 deletions(-) create mode 100644 packages/adblock/files/99_adblock_migrate_lists.sh diff --git a/packages/adblock/Makefile b/packages/adblock/Makefile index 9cfeef42c..7ccb36cc4 100644 --- a/packages/adblock/Makefile +++ b/packages/adblock/Makefile @@ -62,4 +62,7 @@ define Package/adblock/install gzip -9n $(1)/etc/adblock/adblock.sources endef + $(INSTALL_DIR) $(1)/etc/uci-defaults + $(INSTALL_BIN) ./files/99_adblock_migrate_lists.sh $(1)/etc/uci-defaults/99_adblock_migrate_lists + $(eval $(call BuildPackage,adblock)) diff --git a/packages/adblock/files/99_adblock_migrate_lists.sh b/packages/adblock/files/99_adblock_migrate_lists.sh new file mode 100644 index 000000000..5c42e3f28 --- /dev/null +++ b/packages/adblock/files/99_adblock_migrate_lists.sh @@ -0,0 +1,24 @@ +#!/bin/sh + +# +# Copyright (C) 2026 Nethesis S.r.l. +# SPDX-License-Identifier: GPL-2.0-only +# +# Migrate adblock whitelist/blacklist files to UCI storage. +# Runs once on first boot after upgrade; subsequent boots skip via section check. + +# Skip if already migrated +uci -q get adblock.ns_lists > /dev/null 2>&1 && exit 0 + +uci set adblock.ns_lists=ns_lists + +for type in whitelist blacklist; do + file="/etc/adblock/adblock.${type}" + [ -f "${file}" ] || continue + while IFS= read -r line || [ -n "${line}" ]; do + [ -z "${line}" ] && continue + uci add_list "adblock.ns_lists.${type}=${line}" + done < "${file}" +done + +uci commit adblock diff --git a/packages/adblock/files/adblock.init b/packages/adblock/files/adblock.init index 44e6228f8..83fd681c2 100755 --- a/packages/adblock/files/adblock.init +++ b/packages/adblock/files/adblock.init @@ -26,6 +26,27 @@ if [ -s "${adb_pidfile}" ] && { [ "${action}" = "start" ] || [ "${action}" = "st return 0 fi +f_write_local_lists() { + local whitelist_file='/etc/adblock/adblock.whitelist' + local blacklist_file='/etc/adblock/adblock.blacklist' + local entry + + # Only overwrite files when UCI ns_lists section exists (post-migration) + uci -q get adblock.ns_lists > /dev/null 2>&1 || return 0 + + : > "${whitelist_file}" + eval set -- $(uci -q get adblock.ns_lists.whitelist 2>/dev/null) + for entry; do + printf '%s\n' "${entry}" >> "${whitelist_file}" + done + + : > "${blacklist_file}" + eval set -- $(uci -q get adblock.ns_lists.blacklist 2>/dev/null) + for entry; do + printf '%s\n' "${entry}" >> "${blacklist_file}" + done +} + boot() { [ -s "${adb_pidfile}" ] && : >"${adb_pidfile}" rc_procd start_service @@ -37,6 +58,7 @@ start_service() { if [ "${action}" = "boot" ]; then [ -n "$(uci_get adblock global adb_trigger)" ] && return 0 fi + f_write_local_lists procd_open_instance "adblock" procd_set_param command "${adb_script}" "${@}" procd_set_param pidfile "${adb_pidfile}" @@ -48,6 +70,7 @@ start_service() { } reload_service() { + f_write_local_lists /usr/sbin/ts-dns # configure threat shield dns, if needed rc_procd start_service reload /etc/init.d/firewall restart diff --git a/packages/ns-api/files/ns.threatshield b/packages/ns-api/files/ns.threatshield index be8ed3de1..dc6ae8ddc 100644 --- a/packages/ns-api/files/ns.threatshield +++ b/packages/ns-api/files/ns.threatshield @@ -101,14 +101,34 @@ def write_allow_list(allow_list, file='/etc/banip/banip.allowlist'): f.write('\n') subprocess.run(["/etc/init.d/banip", "reload"], capture_output=True) +def dns_get_local_list(file): + option = 'whitelist' if 'whitelist' in file else 'blacklist' + try: + values = EUci().get('adblock', 'ns_lists', option, list=True) + except Exception: + return [] + ret = [] + for v in values: + parts = v.split('#', 1) + ret.append({'address': parts[0].strip(), 'description': parts[1].strip() if len(parts) > 1 else ''}) + return ret + def dns_write_local_list(local_list, file): - with open(file, 'w') as f: - for x in local_list: - f.write(x['address']) - if x['description']: - f.write(' #' + x['description']) - f.write('\n') - subprocess.run(["/etc/init.d/adblock", "restart"], capture_output=True) + option = 'whitelist' if 'whitelist' in file else 'blacklist' + e_uci = EUci() + e_uci.set('adblock', 'ns_lists', 'ns_lists') + values = tuple( + f"{x['address']} #{x['description']}" if x.get('description') else x['address'] + for x in local_list + ) + if values: + e_uci.set('adblock', 'ns_lists', option, values) + else: + try: + e_uci.delete('adblock', 'ns_lists', option) + except Exception: + pass + e_uci.save('adblock') def write_block_list(block_list): write_allow_list(block_list, '/etc/banip/banip.blocklist') @@ -465,13 +485,13 @@ def dns_edit_settings(e_uci, payload): return {'message': 'success'} def dns_list_allowed(): - return { "data": get_allow_list('/etc/adblock/adblock.whitelist') } + return { "data": dns_get_local_list('/etc/adblock/adblock.whitelist') } def dns_list_blocked(): - return { "data": get_allow_list('/etc/adblock/adblock.blacklist') } + return { "data": dns_get_local_list('/etc/adblock/adblock.blacklist') } def dns_add_allowed(payload): - cur = get_allow_list('/etc/adblock/adblock.whitelist') + cur = dns_get_local_list('/etc/adblock/adblock.whitelist') # extract address from cur list if payload['address'] in [x['address'] for x in cur]: raise ValidationError('address', 'address_already_present', payload['address']) @@ -480,7 +500,7 @@ def dns_add_allowed(payload): return {'message': 'success'} def dns_add_blocked(payload): - cur = get_allow_list('/etc/adblock/adblock.blacklist') + cur = dns_get_local_list('/etc/adblock/adblock.blacklist') # extract address from cur list if payload['address'] in [x['address'] for x in cur]: raise ValidationError('address', 'address_already_present', payload['address']) @@ -489,7 +509,7 @@ def dns_add_blocked(payload): return {'message': 'success'} def dns_edit_allowed(payload): - cur = get_allow_list('/etc/adblock/adblock.whitelist') + cur = dns_get_local_list('/etc/adblock/adblock.whitelist') if payload['address'] not in [x['address'] for x in cur]: raise ValidationError('address', 'address_not_found', payload['address']) for i in range(len(cur)): @@ -500,7 +520,7 @@ def dns_edit_allowed(payload): return {'message': 'success'} def dns_edit_blocked(payload): - cur = get_allow_list('/etc/adblock/adblock.blacklist') + cur = dns_get_local_list('/etc/adblock/adblock.blacklist') if payload['address'] not in [x['address'] for x in cur]: raise ValidationError('address', 'address_not_found', payload['address']) for i in range(len(cur)): @@ -511,7 +531,7 @@ def dns_edit_blocked(payload): return {'message': 'success'} def dns_delete_allowed(payload): - cur = get_allow_list('/etc/adblock/adblock.whitelist') + cur = dns_get_local_list('/etc/adblock/adblock.whitelist') if payload['address'] not in [x['address'] for x in cur]: raise ValidationError('address', 'address_not_found', payload['address']) # remove address from cur list @@ -523,7 +543,7 @@ def dns_delete_allowed(payload): return {'message': 'success'} def dns_delete_blocked(payload): - cur = get_allow_list('/etc/adblock/adblock.blacklist') + cur = dns_get_local_list('/etc/adblock/adblock.blacklist') if payload['address'] not in [x['address'] for x in cur]: raise ValidationError('address', 'address_not_found', payload['address']) # remove address from cur list