Add package fff-layer3-ipv4snat #26

Closed
ChristianD wants to merge 1 commits from ChristianD/firmware:ipv4snat into master
3 changed files with 92 additions and 1 deletions

View File

@ -0,0 +1,32 @@
include $(TOPDIR)/rules.mk
PKG_NAME:=fff-layer3-ipv4snat
PKG_RELEASE:=1
include $(INCLUDE_DIR)/package.mk
define Package/fff-layer3-ipv4snat
SECTION:=base
CATEGORY:=Freifunk
TITLE:=Freifunk-Franken layer3 configuration with SNAT
URL:=https://www.freifunk-franken.de
DEPENDS:= \
+iptables-mod-nat-extra \
+fff-firewall \
+fff-layer3-config
endef
define Package/fff-layer3-ipv4snat/description
With this package it is possible to make SNAT with IPv4 on the router
endef
define Build/Compile
# nothing
endef
define Package/fff-layer3-ipv4snat/install
$(CP) ./files/* $(1)/
endef
$(eval $(call BuildPackage,fff-layer3-ipv4snat))

View File

@ -0,0 +1,58 @@
# configure-layer3 -c do nothing
# Check if NAT is set
# If NAT is set
if uci -q get gateway.@client[0].nat; then
Review

Mir gefällt die Hierarchie-Änderung hier nicht. Ich fände es wesentlich schöner, wenn man innerhalb der Funktionen Bedingungen ansetzt als die Funktionen anhand einer Bedingung zu definieren.

Mir gefällt die Hierarchie-Änderung hier nicht. Ich fände es wesentlich schöner, wenn man innerhalb der Funktionen Bedingungen ansetzt als die Funktionen anhand einer Bedingung zu definieren.
Review

Geb ich dir mittlerweile recht, ich denke auch das umzudrehen ist besser. Ist in Arbeit

Geb ich dir mittlerweile recht, ich denke auch das umzudrehen ist besser. Ist in Arbeit
if ! peer_ip=$(uci get gateway.meta.peer_ip); then
echo "WARNING: No peer_ip set! For NAT you must set a peer_ip"
Review

Wenn ein Feature konfiguriert ist, was mit der aktuellen Konfiguration nicht umsetzbar ist, unbedingt mit Fehler beenden, damit die Konfiguration nicht fertig läuft.
Außerdem dann "ERROR: " statt "WARNING: "

Wenn ein Feature konfiguriert ist, was mit der aktuellen Konfiguration nicht umsetzbar ist, unbedingt mit Fehler beenden, damit die Konfiguration nicht fertig läuft. Außerdem dann "ERROR: " statt "WARNING: "
fi
# read ipaddr but we need no warning
# the warning come from 30-network-client if this not set
ipaddr=$(uci get gateway.@client[0].ipaddr)
# configure-layer3 -t - reload set the iptables rule not rebootsafe
reload() {
# first we flush the table
iptables -t nat --flush
Review

Die ganze Tabelle zu flushen ist für mich hier ein no-go. Damit flusht man sich potentiell sehr viel mehr weg, als man eigentlich möchte. Das explodiert, wenn man auch nur eine andere Sache in die nat-Tabelle eintragen möchte.

Entweder wir finden einen Weg die Regel wieder sauber zu finden, oder wir starten die ganze Firewall neu, so dass alle Regeln nochmal sauber angewendet werden - war mir eigentlich viel lieber wäre. Ggf. muss man hier noch die Firewall anpassen.
Vorsicht: Die Schnittstelle unserer Firewall gibt aktuell eigentlich kein apply-before-safe her. Da müsste man wohl noch was bauen, z.B. dass die Firewall noch Zeug aus /tmp included, was nach Reboot weg ist. Vielleicht lohnt es sich hier mal zu gucken wie uci das macht.

Die ganze Tabelle zu flushen ist für mich hier ein no-go. Damit flusht man sich potentiell sehr viel mehr weg, als man eigentlich möchte. Das explodiert, wenn man auch nur eine andere Sache in die nat-Tabelle eintragen möchte. Entweder wir finden einen Weg die Regel wieder sauber zu finden, oder wir starten die ganze Firewall neu, so dass alle Regeln nochmal sauber angewendet werden - war mir eigentlich viel lieber wäre. Ggf. muss man hier noch die Firewall anpassen. Vorsicht: Die Schnittstelle unserer Firewall gibt aktuell eigentlich kein apply-before-safe her. Da müsste man wohl noch was bauen, z.B. dass die Firewall noch Zeug aus /tmp included, was nach Reboot weg ist. Vielleicht lohnt es sich hier mal zu gucken wie uci das macht.
Review

Prinzipiell gebe ich dir erstmal recht, flush ist unschön.
Anderseits muss ich aber sicherstellen, das nicht irgendeine rule dabei steht die meine rule unbrauchbar macht sonst funktioniert im worst case das ganze Paket hier nicht. Man könnte also sagen der flush ist nötig damit ich einen sicheren Ausgangszustand habe. Vermutlich ist aber auch hier flush wieder der falsche Weg.

Beim OpenWRT werden die uci firewall in /etc/config/network geschrieben allerdings ist dafür ein firewall Paket nötig das wir nicht haben (glaube ich zumindest, müsste ich jetzt nochmal gucken).

Die ganze Firewall neu starten ist vielleicht keine schlechte Idee, darüber denke ich mal nach vllt. kann man da was brauchbares draus machen.

Dein apply-bevor-safe "Problem" verstehe ich gerade nicht.

Prinzipiell gebe ich dir erstmal recht, flush ist unschön. Anderseits muss ich aber sicherstellen, das nicht irgendeine rule dabei steht die meine rule unbrauchbar macht sonst funktioniert im worst case das ganze Paket hier nicht. Man könnte also sagen der flush ist nötig damit ich einen sicheren Ausgangszustand habe. Vermutlich ist aber auch hier flush wieder der falsche Weg. Beim OpenWRT werden die uci firewall in /etc/config/network geschrieben allerdings ist dafür ein firewall Paket nötig das wir nicht haben (glaube ich zumindest, müsste ich jetzt nochmal gucken). Die ganze Firewall neu starten ist vielleicht keine schlechte Idee, darüber denke ich mal nach vllt. kann man da was brauchbares draus machen. Dein apply-bevor-safe "Problem" verstehe ich gerade nicht.
Review

Dein apply-bevor-safe "Problem" verstehe ich gerade nicht.

Wenn du die Firewallregeln schon beim Test anlegst (configuregateway -t), dann ist die Firewallregeln schon rebootsicher abgelegt. Alle anderen Einstellungen gehen verloren, wenn man kein commit macht und dann rebootet.

Daher müsste man die Firewall so erweitern, dass diese auch irgendwoher nicht-persistente Regeln lesen kann. Beispielsweise aus der Ramdisk, also /tmp.

> Dein apply-bevor-safe "Problem" verstehe ich gerade nicht. Wenn du die Firewallregeln schon beim Test anlegst (configuregateway -t), dann ist die Firewallregeln schon rebootsicher abgelegt. Alle anderen Einstellungen gehen verloren, wenn man kein commit macht und dann rebootet. Daher müsste man die Firewall so erweitern, dass diese auch irgendwoher nicht-persistente Regeln lesen kann. Beispielsweise aus der Ramdisk, also /tmp.
# and load the new settings
for ip in $ipaddr; do
iptables -t nat -A POSTROUTING -s $ip -j SNAT --to-source $peer_ip
done
}
# configure-layer3 -a - apply write iptables rule to firewall and set rule again
# because it is possible that the rule is not set here but we need it here
apply() {
iptables -t nat --flush
rm -rf /usr/lib/firewall.d/30-NAT
for ip in $ipaddr; do
iptables -t nat -A POSTROUTING -s $ip -j SNAT --to-source $peer_ip
Review

reload() wird automatisch auch nach einem apply() aufgerufen. Hier gehört also nur rein, was tatsächlich das "applien" betrifft.

reload() wird automatisch auch nach einem apply() aufgerufen. Hier gehört also nur rein, was tatsächlich das "applien" betrifft.
Review

Hm ich stolpere hier gerade drüber:

Wenn ich Testmode verwende, will ich die rule nur in /tmp haben also muss in reload() ein:

echo "iptables -t nat -A POSTROUTING -s $ip -j SNAT --to-source $peer_ip" >> /tmp/firewall.d/30-NAT

rein.

in applien soll die rule nach /usr/lib/firewall.d/30-NAT einfügen, da danach aber wieder ein reload() aufgerufen wird, ist sie automatisch auch wieder im /tmp drinnen und damit doppelt vorhanden wenn man die Firewall neu startet. Irgendwie mäh oder überseh ich etwas?

Hm ich stolpere hier gerade drüber: Wenn ich Testmode verwende, will ich die rule nur in /tmp haben also muss in reload() ein: echo "iptables -t nat -A POSTROUTING -s $ip -j SNAT --to-source $peer_ip" >> /tmp/firewall.d/30-NAT rein. in applien soll die rule nach /usr/lib/firewall.d/30-NAT einfügen, da danach aber wieder ein reload() aufgerufen wird, ist sie automatisch auch wieder im /tmp drinnen und damit doppelt vorhanden wenn man die Firewall neu startet. Irgendwie mäh oder überseh ich etwas?
Review

Vielleicht erledigt sich das aber damit #26 (comment)

Ich werde erstmal die uci Idee weiter verfolgen bevor ich nochmal an /tmp bastel

Vielleicht erledigt sich das aber damit https://git.freifunk-franken.de/freifunk-franken/firmware/pulls/26#issuecomment-955 Ich werde erstmal die uci Idee weiter verfolgen bevor ich nochmal an /tmp bastel
echo "iptables -t nat -A POSTROUTING -s $ip -j SNAT --to-source $peer_ip" >> /usr/lib/firewall.d/30-NAT
done
}
# If NAT is not set
else
# configure-layer3 -t - reload flush the iptables
reload() {
# we only flush the table
iptables -t nat --flush
}
# configure-layer3 -a - apply flush iptables rule and delete firewall rule
apply() {
# we flush the table
iptables -t nat --flush
# and delete the firewall
rm -rf /usr/lib/firewall.d/30-NAT
# nobody need NAT we win! \o/
}
fi
# revert is the same whether NAT set or not
revert() {
# first we flush the table
iptables -t nat --flush
# and load the old settings
. /usr/lib/firewall.d/30-NAT
}

View File

@ -1,7 +1,7 @@
include $(TOPDIR)/rules.mk
PKG_NAME:=fff-layer3
PKG_RELEASE:=7
PKG_RELEASE:=8
PKG_BUILD_DIR:=$(BUILD_DIR)/fff-layer3
@ -17,6 +17,7 @@ define Package/fff-layer3
+fff-boardname \
+fff-dhcp \
+fff-layer3-config \
+fff-layer3-ipv4snat \
+fff-network \
+fff-ra \
+fff-wireguard \