fix: revert to using #Access for firewall targets to prevent routing loops
This commit is contained in:
@@ -13,7 +13,7 @@ Berikut adalah versi yang sudah diperbaiki dan ditingkatkan secara menyeluruh be
|
||||
| **LOG setelah ACCEPT** | `LOG` ditempatkan di akhir chain setelah `ACCEPT`, sehingga tidak pernah match | Dipindah: `LOG` ditempatkan sebelum `ACCEPT` final, atau gunakan target `LOG` + return |
|
||||
| **Race condition lock** | `flock` di subshell `while read` pipe bisa kehilangan lock | Lock dipindah ke main shell, subshell hanya baca |
|
||||
| **No rollback** | Jika `policy-engine` gagal di tengah, rule setengah jadi | Ditambahkan atomic swap dengan backup chain |
|
||||
| **No IP validation** | Target IP langsung dipakai tanpa validasi | Ditambahkan regex IPv4/IPv6 validation |
|
||||
| **No IP validation** | IP dari `#Access` langsung dipakai tanpa validasi | Ditambahkan regex IPv4/IPv6 validation |
|
||||
|
||||
---
|
||||
|
||||
@@ -426,7 +426,7 @@ main() {
|
||||
echo '{"clients":{}}' > "$tmp_policy"
|
||||
|
||||
# Parse peers from wg0.conf
|
||||
# AWK extracts client IP and target IPs from AllowedIPs per [Peer] block
|
||||
# AWK extracts IP and #Access comment per [Peer] block
|
||||
local parse_errors=0
|
||||
|
||||
awk '
|
||||
@@ -438,18 +438,12 @@ main() {
|
||||
split($i,a," = ")
|
||||
gsub(/ /,"",a[2])
|
||||
split(a[2],b,",")
|
||||
|
||||
# First IP is the client IP
|
||||
split(b[1],c,"/")
|
||||
ip=c[1]
|
||||
|
||||
# The rest of the IPs are access targets
|
||||
access_arr=""
|
||||
for(j=2;j<=length(b);j++) {
|
||||
if(access_arr=="") access_arr = b[j]
|
||||
else access_arr = access_arr "," b[j]
|
||||
}
|
||||
access = access_arr
|
||||
if($i ~ /^#Access/) {
|
||||
sub(/^#Access[ \t]+/, "", $i)
|
||||
access=$i
|
||||
}
|
||||
}
|
||||
if(ip!="" && ip!="0.0.0.0" && ip!="::") {
|
||||
@@ -1017,15 +1011,18 @@ PostDown = /usr/local/bin/wg-policy-cleanup.sh
|
||||
|
||||
[Peer]
|
||||
PublicKey = <client1-pubkey>
|
||||
AllowedIPs = 10.0.0.2/32, 192.168.1.10/32, 192.168.12.0/24, 192.168.12.2/32
|
||||
AllowedIPs = 10.0.0.2/32
|
||||
#Access 192.168.1.10/32;192.168.12.0/24,192.168.12.2/32
|
||||
|
||||
[Peer]
|
||||
PublicKey = <client2-pubkey>
|
||||
AllowedIPs = 10.0.0.3/32, 10.0.0.1/32
|
||||
AllowedIPs = 10.0.0.3/32
|
||||
#Access 10.0.0.1/32
|
||||
|
||||
[Peer]
|
||||
PublicKey = <client3-pubkey>
|
||||
AllowedIPs = 10.0.0.4/32
|
||||
#Access
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
@@ -5,10 +5,10 @@
|
||||
- **Data Flow:** `wg0.conf` -> `wg-sync-policy.sh` -> `policy.json` -> `wg-policy-engine.sh` -> `iptables`/`ipset`
|
||||
- **File Watcher:** `wg-sync-watch.sh` monitors `wg0.conf` via `inotifywait` and debounces changes to re-run the sync and engine.
|
||||
|
||||
## Critical Parsing Rules (User Override)
|
||||
- **Target IPs Parsing:** The script MUST NOT rely on `#Access` comments for client target isolation rules.
|
||||
- Instead, target allowed IPs should be parsed directly from the client's `AllowedIPs` list in the peer configuration (e.g., `AllowedIPs = 172.20.8.0/24,172.20.10.91/32,...`).
|
||||
- Note: Usually `AllowedIPs` defines the client's source IP (often a `/32`), but in this specific setup logic, multiple IPs listed in a peer's `AllowedIPs` act as the allowed destinations/access targets for that peer.
|
||||
## Critical Parsing Rules & Design Constraints
|
||||
- **Target IPs Parsing (`#Access`):** The firewall script uses the custom `#Access` comment in `wg0.conf` to define egress/firewall whitelists for clients.
|
||||
- **Why `#Access` is mandatory:** WireGuard's native `AllowedIPs` on a Server dictates *routing* towards the client. If we put target destinations in the Server's `AllowedIPs`, the Server would wrongly route traffic destined for those IPs *into* the client tunnel. Therefore, a custom `#Access` comment is the only correct way to define firewall whitelist destinations without breaking WireGuard's Cryptokey Routing.
|
||||
- **Do not remove `#Access`:** Future agents MUST NOT attempt to refactor the script to parse targets from `AllowedIPs`. It is architecturally incorrect for this use case.
|
||||
|
||||
## Testing & Verifying
|
||||
- `wg-policy-ctl status`: Check the overall health, including interface status, JSON validity, lock files, and iptables rules counts.
|
||||
|
||||
+1
-1
@@ -40,7 +40,7 @@ main() {
|
||||
echo '{"clients":{}}' > "$tmp_policy"
|
||||
|
||||
# Parse peers from wg0.conf
|
||||
# AWK extracts client IP and target IPs from AllowedIPs per [Peer] block
|
||||
# AWK extracts IP and #Access comment per [Peer] block
|
||||
local parse_errors=0
|
||||
|
||||
awk '
|
||||
|
||||
Reference in New Issue
Block a user