Skip to content

Коррекция правил iptables #116

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

Closed
AltGrF13 opened this issue Mar 27, 2024 · 10 comments
Closed

Коррекция правил iptables #116

AltGrF13 opened this issue Mar 27, 2024 · 10 comments

Comments

@AltGrF13
Copy link
Contributor

AltGrF13 commented Mar 27, 2024

Есть куча скриптов в opt/etc/ndm/netfilter.d, которые срабатывают при перезапуске роутера целиком или какого-то из его интерфейсов (например, в веб-морде что-то изменили). В 100-dns-local идёт вызов ip4_firewall_dns_rules_set из opt/etc/ndm/ndm:129, где:

  1. Правила iptables без протокола, т.е. ALL. Т.е. туда полетят и ICMP (как минимум), а SS умеет работать только с TCP и UDP. В общем, для SS нельзя создавать правила iptables без -p, посыл коммита 6e5db2e2dda07537aa1d4a285b2b4cf09a31d52e не верен. Как минимум не откачено ещё в:
  • ip4_firewall_vpn_mark
  • ip4_firewall_mark_rules_tcp_udp_on
  • ip4tbl_flush_rm_match_set
  1. В if остался grep закомментированного protocol, после вышеназванного коммита условие не срабатывает никогда (более того, в логе роутера постоянный спам ошибки), для подключения SS этих правил DNS в iptables больше нет (хотя несколько лет подряд КВАСом для SS они создавались всегда)
iptables -A PREROUTING -w -t nat -i br0 -p tcp --dport 53 -j DNAT --to 192.168.1.1
iptables -A PREROUTING -w -t nat -i br0 -p udp --dport 53 -j DNAT --to 192.168.1.1

И знаете… пока прекрасно работает без этого. Если я правильно их понимаю, они были нужны, если клиенты WiFi указывали кастомный нешифрованный DNS, то обход продолжал работать. Но если человек так поступил, то сам себе злобный Буратино — в доке описано так не поступать; да и смущает разное поведение на шифрованный и нешифрованный DNS из-за них.

В общем, предлагаю этот метод откатить (к использованию protocol) и временно задокументировать целиком. Если жалоб не будет, то при очередном рефакторинге почистить и его тело, и вызов в 100-dns-local.

ip4_firewall_dns_rules_set() {
#	interface=$(get_local_inface)
	local_ip=$(get_router_ip)

#	for protocol in tcp udp ; do
		#	Если не заданы аргументы, то ничего не выполняем
#		if [ -n "${interface}" ] && [ -n "${local_ip}" ]; then
			# если правила для tcp есть, то пропускаем их добавление
#			if ! ip4save | grep -q "${protocol} \-\-dport ${DNS_PORT} \-j DNAT" ; then
#				log_warning "Подключаем правила для корректной работы DNS трафика через 53 порт:"
#				log_warning "Интерфейс: ${interface}, IP: ${local_ip}, протокол: ${protocol}."
#				ip4tables PREROUTING -w -t nat -i "${interface}" -p "${protocol}" --dport ${DNS_PORT} -j DNAT --to "${local_ip}" &>/dev/null \
#					|| error "[${FUNCNAME}] Возникла ошибка в функции при установке правил iptables."
#			fi
#		else
#			log_error "При вызове ip4_firewall_dns_rules_set не были заданы обязательные аргументы."
#		fi
#	done
}
@qzeleza
Copy link
Owner

qzeleza commented Mar 28, 2024

Я правильно Вас понял, что Вы предлагаете полностью удалить ip4_firewall_dns_rules_set?

@AltGrF13
Copy link
Contributor Author

AltGrF13 commented Mar 28, 2024

Фактически, да.

  1. В комментариях к вызову этой функции указано, что она для работы SS. Что как раз мой (хорошо изученный) случай.
  2. После указанного январского коммита этот код всё равно не отрабатывал; что и удивило меня — привычных 2 правил в iptables нет.
  3. Но обход прекрасно работал, это заставило внимательнее присмотреться к привычному. В итоге мы видим, что создавались 2 правила для основного домашнего WiFi; которые, встречая трафик на не зашифрованный DNS, перенаправляют его на роутер (если они шли не к нему, фактически). Ну и у этого кода, получается, довольно слабая позиция: лишь один случай DNS, лишь одна подсеть.
  4. Но, опять же, не проверялось с adguard (вдруг там какой-то хитрый механизм, который я пока не понимаю). Или вдруг влияет использование роутером во вне шифрованного DNS? Все случаи объять невозможно.
  5. Но, заметьте, когда мы расшаривали основной SS другим подсетям, то мы всегда тащили 2 правила для роутинга трафика. А от этих DNS-правил для других подсетей отказались (фактически, однажды мы их уже выкинули для других подсетей — даже Ваш коммент на эту тему могу найти).

@qzeleza
Copy link
Owner

qzeleza commented Mar 28, 2024

Добро, спасибо, после проверки - включу изменения в следующий релиз.

@AltGrF13
Copy link
Contributor Author

AltGrF13 commented Mar 29, 2024

В первом пункте ещё описано, что SS нельзя создавать правила iptables без -p. И что в ip4tbl_flush_rm_match_set код откачен так и не был. Там сейчас вот такой блок

#		for prot in tcp udp; do
#		ip4save | grep "${route}" | grep "${chain}" | grep -q "${prot}" && {
		ip4save | grep "${route}" | grep -q "${chain}" && {
			if [ -n "${interface}" ] && [ -n "${proxy_port}" ] ; then
#					Для shadowsocks
				iptab -t "${table}" -i "${interface}" -D "${route}" -p "${prot}" -m set --match-set ${table_name} dst -j "${chain}" --to-port "${proxy_port}" &>/dev/null
				iptab -t "${table}" -i "${interface}" -D "${route}" -p "${prot}" -m set --match-set ${table_name} dst -j "${chain}" --to-port "${proxy_port}" &>/dev/null
			else
#					Для VPN
#				iptab -t "${table}" -D "${route}" -p "${prot}" -m set --match-set ${table_name} dst -j "${chain}" &>/dev/null
				iptab -t "${table}" -D "${route}" -m set --match-set ${table_name} dst -j "${chain}" &>/dev/null
			fi
		}
#		done

Как видите, переменная prot закомменчена, а правила в ветке shadowsocks с ней (какими они и должны быть). Сейчас эти правила не очищаются, выходит. И сыпят ошибкой в лог.

@AltGrF13
Copy link
Contributor Author

AltGrF13 commented Mar 29, 2024

опять же, не проверялось с adguard (вдруг там какой-то хитрый механизм, который я пока не понимаю)

А к этому подозрению бы добавил; что, может эти 2 правила нужны, когда основное соединение VPN (и комментарий в коде, что это лишь для SS, ошибочен)? Ибо SS+dnsmasq точно ничего такого не требует, всё прекрасно работает.

@AltGrF13
Copy link
Contributor Author

AltGrF13 commented Mar 29, 2024

Ну и раз залезли в эту область кода, то немного странная логика в триггерных файлах opt/etc/ndm/netfilter.d

100-dns-local

  • ip4_firewall_dns_rules_set создаёт 2 этих странных DNS-правила, её вызов я и предлагал закомментировать.

  • set_guest_nets_rules добавляет обход гостевых, если они есть (вызывая внутри себя ip4_add_guest_to_ssr_network или ip4_add_guest_to_vpn_network). Почему обход гостевых вообще в DNS-файле, который ещё и стартует раньше остальных?

100-proxy-redirect

  • ip4_firewall_ssr_prune очищает правила SS

  • ip4_firewall_set_ssr_rules возвращает CHAIN-правила и обход для домашнего WiFi

  • Разве не логично именно тут иметь возврат гостевых правил через вызов конкретно для SS ip4_add_guest_to_ssr_network?

100-vpn-mark

			if fastnet_enabled ; then
				ip4_firewall_vpn_mark &> /dev/null
			else
				ip4_firewall_mark_rules_tcp_udp_on &> /dev/null
			fi

Но у нас уже есть готовая функция ip4_mark_vpn_network, в которой делается тоже самое + накидывание гостевых правил.

Итого, предлагается:

  1. Из 100-dns-local выкинуть накидывание гостевых правил (и файл не тот, и очерёдность).
  2. В 100-proxy-redirect добавить накидывание гостевых правил SS последним действием (это файл SS, логично здесь это и держать).
  3. В 100-vpn-mark тоже добавить воссоздание гостевых правил VPN последним действием, путём вызова общей функции, в которой есть все действия.

@qzeleza
Copy link
Owner

qzeleza commented Mar 31, 2024

Хорошо, спасибо, посмотрю

@qzeleza
Copy link
Owner

qzeleza commented Apr 14, 2024

изменения внесены в 1.1.8 r1
прошу дать обратную связь.

@AltGrF13
Copy link
Contributor Author

AltGrF13 commented Apr 15, 2024

После обновления были проверены (что на сайтах из списка обхода подключение через прокси, а на обычные — напрямую) домашний WiFi, гостевой, IKEv2 при основном подключении SS. Везде всё хорошо.

В логе ошибка тоже пропала.

Вопрос можно закрывать.

@qzeleza
Copy link
Owner

qzeleza commented May 20, 2024

закрываю, раз проблема ушла

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