continuous-integration/drone/push Build is passing
- Ignore "endpoint already exists" error in ConnectContainer on re-reconciliation - Improve iptables comment generation to avoid trailing dashes - Enhance DNAT rule logic: try multiple selectors and fall back to host rules - Add missing "-t nat" flag in InsertPreroutingRuleOnInterface
614 lines
29 KiB
Markdown
614 lines
29 KiB
Markdown
# CLAUDE.md — firewall_containers
|
||
|
||
Comprehensive reference for AI assistants and developers working on this project.
|
||
|
||
---
|
||
|
||
## Table of Contents
|
||
|
||
1. [Project Overview](#project-overview)
|
||
2. [Section 1: Shell Script (`firewall/firewall-add`)](#section-1-shell-script-firewallfiirewall-add)
|
||
3. [Section 2: Go Implementation (`network-go/`)](#section-2-go-implementation-network-go)
|
||
4. [Section 3: Divergencies and Bugs](#section-3-divergencies-and-bugs)
|
||
|
||
---
|
||
|
||
## Project Overview
|
||
|
||
This repository contains two implementations of the same firewall/network management system for Docker containers:
|
||
|
||
- **`firewall/firewall-add`** — the original shell script, driven by environment variables, that manages iptables rules for Docker containers using `nsenter` and the Docker CLI.
|
||
- **`network-go/`** — a Go rewrite of the shell script, driven by a JSON config file (`networks.json`), that manages Docker networks, container connections, and iptables rules using the Docker SDK and `nsenter`.
|
||
|
||
The Go implementation is a **transformation** of the shell script: it replaces ad-hoc environment variable inputs and Docker CLI calls with a structured JSON config, the Docker SDK, and typed Go packages.
|
||
|
||
### CI/CD
|
||
|
||
Drone CI (`.drone.yml`) runs on `master` branch pushes:
|
||
1. `test` — runs `go test ./...` in `network-go/`
|
||
2. `build` — compiles the Go binary
|
||
3. `build-multiarch` — builds and pushes a multi-arch Docker image (`linux/amd64`, `linux/arm64`) to `registry.dev.format.hu/network-go`
|
||
4. `push-to-dockerhub` — pushes to `safebox/network-go` on Docker Hub
|
||
|
||
---
|
||
|
||
## Section 1: Shell Script (`firewall/firewall-add`)
|
||
|
||
### Purpose
|
||
|
||
`firewall/firewall-add` is a POSIX-compatible shell script that configures iptables rules for Docker container networking. It is invoked as a one-shot command, driven entirely by environment variables. It supports:
|
||
|
||
- **Host firewall rules** (FORWARD chain ACCEPT rules)
|
||
- **PREROUTING DNAT** (port forwarding into containers, either on the host or inside a container namespace via `nsenter`)
|
||
- **POSTROUTING MASQUERADE** (NAT for source/target subnets)
|
||
- **IP route injection** into container network namespaces via `nsenter`
|
||
- **Name resolution** from a hosts file or live Docker container inspection
|
||
|
||
### Environment Variables
|
||
|
||
| Variable | Description |
|
||
|---|---|
|
||
| `DEBUG` | Set to `1` to print debug output to stdout; otherwise logs to `/var/log/iptables.log` |
|
||
| `ROUTE` | If `"true"`, runs `ip route` injection mode and exits |
|
||
| `HOST` | If `"true"`, uses host iptables (`/sbin/iptables -t nat`) instead of nsenter |
|
||
| `PREROUTING` | If `"true"`, applies PREROUTING DNAT rules |
|
||
| `POSTROUTING` | If `"true"`, applies POSTROUTING MASQUERADE rules |
|
||
| `CHAIN` | iptables chain to operate on (e.g. `FORWARD`, `DOCKER-USER`) |
|
||
| `NAME` | Container name (used for nsenter PID lookup and comment prefix) |
|
||
| `COMMENT` | Comment string appended to iptables rules; combined as `NAME-COMMENT` |
|
||
| `TYPE` / `PROTOCOL` | Protocol (`tcp` by default) |
|
||
| `OPERATION` | If `"DELETE"`, deletes matching rules instead of inserting |
|
||
| `SOURCE` | Source name(s) or IP(s) (space-separated) |
|
||
| `SOURCE_IP` | Explicit source IP (skips name resolution) |
|
||
| `SOURCE_IP_N` | Indexed source IPs (e.g. `SOURCE_IP_1`, `SOURCE_IP_2`) |
|
||
| `SOURCE_PORT` / `SOURCE_PORT_N` | Source port(s) |
|
||
| `SOURCE_IFACE` | Source network interface (used instead of IP for PREROUTING) |
|
||
| `TARGET` | Target name(s) or IP(s) (space-separated) |
|
||
| `TARGET_IP` | Explicit target IP (skips name resolution) |
|
||
| `TARGET_IP_N` | Indexed target IPs |
|
||
| `TARGET_PORT` / `TARGET_PORT_N` | Target port(s) |
|
||
| `STRICK_CHECK` | If set, uses live Docker container inspection instead of hosts file |
|
||
| `SERVICE_FILES` | Space-separated paths to JSON service definition files (used with `STRICK_CHECK`) |
|
||
| `HOST_FILE` | Path to hosts file for name resolution (default: `/etc/dns/hosts.local`) |
|
||
| `RETRIES_NUMBER` | Number of retries for container name resolution (default: `2`) |
|
||
| `ROLES` | Filter containers by Docker label `roles` |
|
||
| `NETWORK_N` / `GATEWAY_N` | Indexed network/gateway pairs for `ip route` injection |
|
||
| `EXTRA_OPTIONS` | Extra CLI arguments; if contains `"debug"`, enables debug mode |
|
||
|
||
### Execution Flow
|
||
|
||
```
|
||
Start
|
||
│
|
||
├─ [ROUTE=true] → ip_route() → nsenter into container → ip route add → exit
|
||
│
|
||
├─ Resolve SOURCE names/IPs → SOURCE_IP_1..N
|
||
├─ Resolve TARGET names/IPs → TARGET_IP_1..N
|
||
│
|
||
├─ echo 1 > /proc/sys/net/ipv4/ip_forward
|
||
│
|
||
├─ Detect iptables binary:
|
||
│ iptables-legacy (if DOCKER-USER chain exists) or iptables
|
||
│
|
||
└─ Nested loops: source_ip × source_port × target_ip × target_port
|
||
│
|
||
├─ [PREROUTING=true or POSTROUTING=true or HOST=true]
|
||
│ ├─ [HOST=true] → use /sbin/iptables -t nat
|
||
│ └─ [else] → use nsenter into container → /sbin/iptables-legacy -t nat
|
||
│ ├─ [PREROUTING=true] → prerouting()
|
||
│ └─ [POSTROUTING=true] → postrouting()
|
||
│
|
||
└─ [else] → Host firewall mode
|
||
├─ Ensure ESTABLISHED,RELATED ACCEPT rule in CHAIN
|
||
├─ Build IPTABLES_OPTIONS and GREP_OPTIONS from SOURCE_IP/PORT, TARGET_IP/PORT
|
||
├─ Delete old matching rules
|
||
└─ [OPERATION=DELETE] → delete by COMMENT
|
||
[else] → insert ACCEPT rule
|
||
```
|
||
|
||
### Name Resolution (`name_resolver`)
|
||
|
||
Two modes controlled by `STRICK_CHECK`:
|
||
|
||
**Without `STRICK_CHECK`** (hosts file mode):
|
||
- Greps `$HOST_FILE` (default `/etc/dns/hosts.local`) for the name as a whole word
|
||
- Extracts the first field (IP address) from matching lines
|
||
|
||
**With `STRICK_CHECK`** (live Docker mode):
|
||
- Strips the suffix after the first `-` from the name
|
||
- Runs `docker ps --format '{{.Names}}\t{{.Status}}' | grep Up | grep $D"-"` to find running containers
|
||
- Optionally filters by `ROLES` label via `docker inspect`
|
||
- Counts running containers vs expected count from `SERVICE_FILES` JSON
|
||
- Retries up to `RETRIES_NUMBER` times (with 1-second sleep) if not enough containers are up
|
||
- Extracts IPs via `docker inspect -f '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}'`
|
||
|
||
### IP Handling
|
||
|
||
- If a source/target token matches the pattern `[0-9]*.[0-9]*.[0-9]*.[0-9]*`, it is treated as a literal IP
|
||
- If the last octet is `0`, it is automatically converted to a `/24` CIDR (e.g. `10.0.0.0` → `10.0.0.0/24`)
|
||
- Multiple IPs are stored in indexed variables (`SOURCE_IP_1`, `SOURCE_IP_2`, etc.)
|
||
- If only one IP is resolved, it is also stored in `SOURCE_IP` / `TARGET_IP`
|
||
|
||
### Port Pairing Logic
|
||
|
||
- Counts `SOURCE_PORT_N` and `TARGET_PORT_N` variables
|
||
- If counts are equal, ports are treated as **pairs**: `SOURCE_PORT_1` maps to `TARGET_PORT_1`, etc.
|
||
- If counts differ, all combinations are iterated (Cartesian product)
|
||
|
||
### PREROUTING Function
|
||
|
||
Inserts a DNAT rule in the `PREROUTING` chain:
|
||
- **Interface mode** (`SOURCE_IFACE` set): `-i $SOURCE_IFACE --dport $SOURCE_PORT -j DNAT --to $TARGET_IP:$TARGET_PORT`
|
||
- **IP mode** (`SOURCE_IP` set): `-d $SOURCE_IP --dport $SOURCE_PORT -j DNAT --to $TARGET_IP:$TARGET_PORT`
|
||
- Deletes previous matching rules before inserting (idempotent)
|
||
|
||
### POSTROUTING Function
|
||
|
||
Inserts MASQUERADE rules in the `POSTROUTING` chain:
|
||
- For `SOURCE_IP`: derives `/24` subnet, inserts `-s $SOURCE_IP_FOR_POSTROUTING --sport $SOURCE_PORT -j MASQUERADE`
|
||
- For `TARGET_IP`: derives `/24` subnet, inserts `-d $TARGET_IP_FOR_POSTROUTING --dport $TARGET_PORT -j MASQUERADE`
|
||
- Deletes previous matching rules before inserting
|
||
|
||
### Route Injection (`ip_route`)
|
||
|
||
When `ROUTE=true`:
|
||
- Reads `NETWORK_N` / `GATEWAY_N` indexed environment variables
|
||
- Uses `nsenter -t $(docker inspect --format {{.State.Pid}} $NAME) -n -- ip route add $NETWORK/24 via $GATEWAY`
|
||
|
||
### iptables Binary Detection
|
||
|
||
```sh
|
||
if /usr/sbin/iptables-legacy -L | grep DOCKER-USER; then
|
||
IPTABLES="/usr/sbin/iptables-legacy"
|
||
else
|
||
IPTABLES="/usr/sbin/iptables"
|
||
fi
|
||
```
|
||
|
||
### `iptables-wrapper-installer.sh`
|
||
|
||
A Kubernetes-originated utility script (Apache 2.0 licensed) that installs an `iptables-wrapper` shim. At container startup, the wrapper detects whether the host uses `iptables-nft` or `iptables-legacy` (by inspecting Kubernetes-specific mangle table chains) and redirects all `iptables` calls to the correct backend. Supports Fedora-style (`alternatives`), Debian-style (`update-alternatives`), and fallback (`symlink`) installation modes.
|
||
|
||
---
|
||
|
||
## Section 2: Go Implementation (`network-go/`)
|
||
|
||
### Purpose
|
||
|
||
`network-go` is a long-running daemon that replaces the one-shot shell script. Instead of environment variables, it reads a structured JSON config file (`networks.json`) and continuously watches it for changes, reconciling Docker networks, container connections, and iptables rules on every change.
|
||
|
||
### Architecture
|
||
|
||
```
|
||
main.go
|
||
└─ Loads config, creates Docker client + iptables manager
|
||
└─ Creates Orchestrator (firewall package)
|
||
└─ Runs initial ReconcileAll()
|
||
└─ Starts FileWatcher (watcher package) → triggers ReconcileAll() on change
|
||
└─ Waits for SIGINT/SIGTERM
|
||
|
||
firewall/orchestrator
|
||
├─ reconcileNetworks() → docker.EnsureNetwork()
|
||
├─ reconcileIPs() → docker.FindContainerName() + docker.ConnectContainer()
|
||
└─ reconcilePolicies()
|
||
├─ "from" policy → iptables.InsertForwardAccept()
|
||
└─ "nat: dnat" → docker.GetContainerPID() + iptables.InsertPreroutingRuleInContainer()
|
||
(fallback: iptables.InsertPreroutingRuleOnInterface())
|
||
```
|
||
|
||
### Package Reference
|
||
|
||
#### `config`
|
||
|
||
Parses `/etc/user/config/networks.json` into typed structs.
|
||
|
||
| Type | Fields |
|
||
|---|---|
|
||
| `NetworksConfig` | `Networks map[string]NetworkConfig`, `IPs map[string]IPConfig`, `Policies []PolicyConfig` |
|
||
| `NetworkConfig` | `NetworkName`, `Subnet`, `Gateway` |
|
||
| `IPConfig` | `IP`, `ContainerName`, `Selector`, `ServiceName` |
|
||
| `PolicyConfig` | `ServiceName`, `ContainerName`, `Selector`, `From`, `To`, `Port`, `Proto`, `Name`, `Iface`, `Nat` |
|
||
| `FirewallRule` | Resolved rule struct (not yet used in reconciliation path) |
|
||
|
||
Helper functions:
|
||
- `ToCIDR(ip)` — converts `x.x.x.0` to `x.x.x.0/24`; passes through existing CIDRs and non-zero IPs unchanged
|
||
- `NetworkPrefix(ip)` — always returns `x.x.x.0/24` from any IP
|
||
- `IsIP(s)` — returns true if `s` is a valid IPv4 address (not a CIDR)
|
||
- `Load(path)` — reads and JSON-unmarshals the config file
|
||
|
||
#### `docker`
|
||
|
||
Docker SDK wrapper implementing `DockerAPI` interface.
|
||
|
||
| Method | Description |
|
||
|---|---|
|
||
| `EnsureNetwork` | Creates a bridge network with subnet/gateway if it doesn't exist |
|
||
| `ConnectContainer` | Connects a container to a network with a static IP |
|
||
| `DisconnectContainer` | Disconnects a container from a network |
|
||
| `InspectContainer` | Returns full container JSON |
|
||
| `WaitForContainerRunning` | Polls container state with 500ms tick, up to a timeout |
|
||
| `GetContainerPID` | Returns `State.Pid` for nsenter operations |
|
||
| `AddRouteInContainer` | Runs `nsenter -t <pid> -n -- ip route add <network> via <gateway>` |
|
||
| `FindContainerName` | Tries exact name → exact selector → prefix match (shell script `grep $D"-"` behavior) |
|
||
|
||
#### `firewall`
|
||
|
||
`Orchestrator` ties all packages together.
|
||
|
||
**`ReconcileAll(ctx, cfg)`**:
|
||
1. `EnsureIPForward()` — writes `1` to `/proc/sys/net/ipv4/ip_forward`
|
||
2. `reconcileNetworks()` — creates missing Docker networks
|
||
3. `reconcileIPs()` — connects containers to networks with static IPs (waits up to 10s for container to be running)
|
||
4. `reconcilePolicies()` — processes each policy:
|
||
- **`from` field present** → `applyForwardRule()`: resolves source/target IPs, selects `DOCKER-USER` (iptables-legacy) or `FORWARD` chain, ensures ESTABLISHED/RELATED rule, inserts ACCEPT rule
|
||
- **`nat: "dnat"`** → `applyNATRule()`: resolves target IP, gets container PID via Docker SDK, inserts PREROUTING DNAT inside container namespace; falls back to interface-based host rule if PID unavailable
|
||
|
||
**Comment construction**: `policy.Name + "-" + policy.ServiceName` (or just `policy.ServiceName` if `Name` is empty) — mirrors the shell script's `NAME="$NAME-$COMMENT"` pattern.
|
||
|
||
#### `iptables`
|
||
|
||
`Manager` implements `IPTablesAPI`. Auto-detects binary at startup.
|
||
|
||
**Binary detection**: runs `/usr/sbin/iptables-legacy -L`, checks output for `DOCKER-USER`.
|
||
|
||
**Key methods**:
|
||
|
||
| Method | Description |
|
||
|---|---|
|
||
| `EnsureIPForward` | `sh -c "echo 1 > /proc/sys/net/ipv4/ip_forward"` |
|
||
| `EnsureEstablishedRelated(chain)` | Inserts `-m state --state established,related -j ACCEPT` if not present |
|
||
| `InsertPreroutingRule` | Host DNAT: deletes old matching rules, inserts `-d $sourceIP --dport $sourcePort -j DNAT --to $targetIP:$targetPort` |
|
||
| `InsertPreroutingRuleOnInterface` | Host DNAT on interface: `-i $iface --dport $sourcePort -j DNAT --to $targetIP:$targetPort` (no pre-deletion) |
|
||
| `InsertPostroutingMasquerade` | Host MASQUERADE: `-s $sourceCIDR --sport $sourcePort -j MASQUERADE` |
|
||
| `InsertPostroutingMasqueradeForTarget` | Host MASQUERADE: `-d $targetCIDR --dport $targetPort -j MASQUERADE` |
|
||
| `InsertForwardAccept` | Host FORWARD ACCEPT: deletes old matching rules, inserts with optional source/target IP/port |
|
||
| `InsertPreroutingRuleInContainer` | Container DNAT via nsenter: same as `InsertPreroutingRule` but inside container namespace |
|
||
| `InsertPostroutingMasqueradeInContainer` | Container MASQUERADE via nsenter |
|
||
| `DeleteLine` | Deletes a rule by line number from a chain |
|
||
|
||
**Rule deletion strategy**: `getLineNumbers()` lists a chain, filters lines containing all grep patterns, returns line numbers in order. `deleteMatchingLines()` iterates in **reverse** (highest line number first) to avoid index shifting — equivalent to the shell script's `tac` + sequential delete.
|
||
|
||
#### `resolver`
|
||
|
||
Resolves names to IPs using `networks.json` only (no Docker CLI, no hosts file).
|
||
|
||
**Resolution order**:
|
||
1. Exact match on `container_name` or `selector` in the `ips` section
|
||
2. Prefix match: if name contains `-`, extracts prefix before first `-` and matches any `container_name` or `selector` starting with that prefix
|
||
|
||
Default retries: `2` (configurable via `SetRetries`).
|
||
|
||
#### `watcher`
|
||
|
||
`FileWatcher` polls a file using MD5 hash comparison at a configurable interval.
|
||
|
||
- `Start()` — computes initial hash, launches goroutine with `time.Ticker`
|
||
- `Stop()` — closes `stopCh` channel to terminate goroutine
|
||
- On hash change: updates `lastHash`, calls `onChange()` callback
|
||
|
||
#### `mock`
|
||
|
||
Test doubles for `DockerAPI` and `IPTablesAPI` interfaces. Tracks call counts and arguments for assertion in tests.
|
||
|
||
### Configuration File (`networks.json`)
|
||
|
||
```json
|
||
{
|
||
"networks": {
|
||
"<key>": {
|
||
"network_name": "...",
|
||
"subnet": "x.x.x.0/24",
|
||
"gateway": "x.x.x.1"
|
||
}
|
||
},
|
||
"ips": {
|
||
"<ip>": {
|
||
"ip": "x.x.x.x",
|
||
"container_name": "...",
|
||
"selector": "...",
|
||
"service_name": "..."
|
||
}
|
||
},
|
||
"policies": [
|
||
{
|
||
"service_name": "...",
|
||
"container_name": "...",
|
||
"selector": "...",
|
||
"from": "...", // triggers FORWARD ACCEPT rule
|
||
"to": "...",
|
||
"port": 80,
|
||
"proto": "tcp"
|
||
},
|
||
{
|
||
"service_name": "...",
|
||
"name": "...",
|
||
"iface": "wg0", // triggers interface-based PREROUTING
|
||
"nat": "dnat", // triggers DNAT rule
|
||
"selector": "...",
|
||
"to": "...",
|
||
"port": 80,
|
||
"proto": "tcp"
|
||
}
|
||
]
|
||
}
|
||
```
|
||
|
||
### Environment Variables
|
||
|
||
| Variable | Default | Description |
|
||
|---|---|---|
|
||
| `NETWORKS_CONFIG_PATH` | `/etc/user/config/networks.json` | Path to config file |
|
||
| `WATCH_PERIOD_SECONDS` | `30` | Polling interval in seconds |
|
||
| `DEBUG` | `false` | Enable debug output (`1` or `true`) |
|
||
| `DOCKER_HOST` | (empty) | Docker daemon socket (auto-used by SDK) |
|
||
|
||
### Docker Run Requirements
|
||
|
||
```bash
|
||
docker run -d \
|
||
--name network-go \
|
||
--network host \
|
||
--pid host \
|
||
--cap-add NET_ADMIN \
|
||
--cap-add SYS_ADMIN \
|
||
-v /var/run/docker.sock:/var/run/docker.sock \
|
||
-v /etc/user/config:/etc/user/config \
|
||
network-go
|
||
```
|
||
|
||
### Build
|
||
|
||
```bash
|
||
# Local
|
||
cd network-go
|
||
go build -o network-go -ldflags="-s -w" .
|
||
|
||
# Docker (multi-arch)
|
||
docker build -t network-go .
|
||
```
|
||
|
||
### Testing
|
||
|
||
```bash
|
||
cd network-go
|
||
go test ./... -count=1 -v
|
||
```
|
||
|
||
Test coverage:
|
||
- `config`: `Load`, `ToCIDR`, `NetworkPrefix`, `IsIP`, `ParseCIDR`, reproducibility
|
||
- `resolver`: exact match, selector match, not found, nil config, `SetConfig`, multiple match (prefix)
|
||
- `firewall`: network creation, container connection, IP forwarding, FORWARD rules (standard + legacy), DNAT with interface fallback, DNAT with container PID, unresolved target, direct IP resolution, `findNetworkForIP`, reproducibility
|
||
- `watcher`: change detection, no-change, multiple changes, stop, nonexistent file, reproducibility
|
||
|
||
---
|
||
|
||
## Section 3: Divergencies and Bugs
|
||
|
||
This section documents all known differences between the shell script (`firewall/firewall-add`) and the Go implementation (`network-go/`), including behavioral divergencies, missing features, and potential bugs.
|
||
|
||
---
|
||
|
||
### D1 — Name Resolution: Hosts File vs. JSON Config
|
||
|
||
**Shell script**: resolves names from `/etc/dns/hosts.local` (or `$HOST_FILE`) by grepping for the name as a whole word. With `STRICK_CHECK`, uses live `docker ps` + `docker inspect` instead.
|
||
|
||
**Go implementation**: resolves names exclusively from the `ips` section of `networks.json` by matching `container_name` or `selector` fields. There is no hosts file lookup and no live Docker container inspection for name resolution.
|
||
|
||
**Impact**: Any deployment that relied on `/etc/dns/hosts.local` for name resolution will silently fail to resolve names in the Go implementation. The Go resolver will log `"no IP found for X in networks.json config"` and return an empty IP, causing the firewall rule to be skipped.
|
||
|
||
---
|
||
|
||
### D2 — `STRICK_CHECK` / Role Filtering Not Implemented
|
||
|
||
**Shell script**: when `STRICK_CHECK` is set, filters containers by the Docker label `roles` (via `docker inspect -f '{{.Config.Labels.roles}}'`). This allows targeting only containers with a specific role in a multi-replica deployment.
|
||
|
||
**Go implementation**: has no concept of `STRICK_CHECK` or role-based filtering. `FindContainerName` does prefix matching but does not inspect Docker labels.
|
||
|
||
**Impact**: Multi-role container deployments that relied on `ROLES` filtering will not work correctly in the Go implementation.
|
||
|
||
---
|
||
|
||
### D3 — POSTROUTING MASQUERADE Not Called in Reconciliation
|
||
|
||
**Shell script**: the `postrouting()` function is called when `POSTROUTING=true`, inserting MASQUERADE rules for both source and target subnets.
|
||
|
||
**Go implementation**: `InsertPostroutingMasquerade` and `InsertPostroutingMasqueradeForTarget` are defined in the `iptables` package and have mock implementations in tests, but **`reconcilePolicies()` never calls them**. There is no policy field (e.g. `"nat": "masquerade"`) that triggers POSTROUTING rules.
|
||
|
||
**Impact**: POSTROUTING MASQUERADE rules are never applied by the Go implementation. Any traffic that required MASQUERADE for correct routing will silently fail.
|
||
|
||
---
|
||
|
||
### D4 — `InsertPreroutingRuleOnInterface` Missing `-t nat`
|
||
|
||
**Shell script**: when `HOST=true`, uses `/sbin/iptables -t nat -I PREROUTING ...`. When using nsenter, uses `/sbin/iptables-legacy -t nat -I PREROUTING ...`. The `-t nat` table is always specified for PREROUTING rules.
|
||
|
||
**Go implementation** (`iptables.go`, `InsertPreroutingRuleOnInterface`):
|
||
```go
|
||
args := []string{
|
||
"-w", "-I", "PREROUTING",
|
||
"-i", iface, ...
|
||
}
|
||
return m.run(args...)
|
||
```
|
||
`m.run()` calls the binary directly **without `-t nat`**. This means the rule is inserted into the `filter` table's PREROUTING chain (which does not exist by default), not the `nat` table.
|
||
|
||
**Impact**: Interface-based PREROUTING DNAT rules inserted via `InsertPreroutingRuleOnInterface` will fail silently or produce an iptables error, as DNAT is only valid in the `nat` table.
|
||
|
||
---
|
||
|
||
### D5 — `InsertPreroutingRule` Uses `-d` (Destination) Instead of Source
|
||
|
||
**Shell script** (`prerouting()` with `SOURCE_IP`):
|
||
```sh
|
||
$IPTABLES -w -I PREROUTING -d $SOURCE_IP -p $PROTOCOL --dport $SOURCE_PORT ... -j DNAT --to $TARGET_IP:$TARGET_PORT
|
||
```
|
||
This matches packets **destined for** `SOURCE_IP` and DNATs them to `TARGET_IP:TARGET_PORT`. The naming is confusing (SOURCE_IP is used as the destination match), but this is the intended behavior for port forwarding to a VIP.
|
||
|
||
**Go implementation** (`InsertPreroutingRule`):
|
||
```go
|
||
"-d", sourceIP,
|
||
```
|
||
This correctly mirrors the shell script's use of `-d`. However, the parameter is named `sourceIP` in the function signature, which is semantically misleading and could cause future callers to pass the wrong value.
|
||
|
||
**Impact**: No functional bug currently, but the misleading parameter name is a maintenance hazard.
|
||
|
||
---
|
||
|
||
### D6 — `InsertPreroutingRuleInContainer` Uses `-d 0.0.0.0/0` (Hardcoded)
|
||
|
||
**Shell script**: when using nsenter for PREROUTING, the rule does not specify a destination IP — it matches all traffic on the port:
|
||
```sh
|
||
nsenter ... /sbin/iptables-legacy -t nat -I PREROUTING -p $PROTOCOL --dport $SOURCE_PORT ... -j DNAT --to $TARGET_IP:$TARGET_PORT
|
||
```
|
||
|
||
**Go implementation** (`firewall.go`, `applyNATRule`):
|
||
```go
|
||
o.iptablesMgr.InsertPreroutingRuleInContainer(pid, "0.0.0.0/0", proto, port, targetIP, port, comment)
|
||
```
|
||
And in `iptables.go`, `InsertPreroutingRuleInContainer`:
|
||
```go
|
||
"-d", sourceIP, // sourceIP = "0.0.0.0/0"
|
||
```
|
||
This adds `-d 0.0.0.0/0` to the rule, which is functionally equivalent to no destination filter, but it is an unnecessary argument that differs from the shell script's behavior and may cause issues with some iptables versions.
|
||
|
||
**Impact**: Minor behavioral difference; functionally equivalent in most cases but adds an unnecessary `-d 0.0.0.0/0` match that the shell script does not include.
|
||
|
||
---
|
||
|
||
### D7 — `OPERATION=DELETE` Not Implemented
|
||
|
||
**Shell script**: when `OPERATION=DELETE`, deletes all rules matching the `COMMENT` from the chain instead of inserting new ones.
|
||
|
||
**Go implementation**: `PolicyConfig` has no `operation` or `delete` field. There is no way to express a delete-only operation in the JSON config. The `DeleteForwardAccept` method exists in the `IPTablesAPI` interface but is never called from `reconcilePolicies()`.
|
||
|
||
**Impact**: Rule deletion via config is not supported. Rules can only be removed by manually running iptables commands.
|
||
|
||
---
|
||
|
||
### D8 — `ROUTE=true` (IP Route Injection) Not Implemented
|
||
|
||
**Shell script**: when `ROUTE=true`, injects `ip route add $NETWORK/24 via $GATEWAY` into a container's network namespace via nsenter. Supports multiple `NETWORK_N`/`GATEWAY_N` pairs.
|
||
|
||
**Go implementation**: `docker.AddRouteInContainer()` is implemented and correctly uses nsenter, but it is **never called from `ReconcileAll()` or `reconcilePolicies()`**. There is no `routes` section in `networks.json` and no policy type that triggers route injection.
|
||
|
||
**Impact**: Container route injection is completely absent from the Go reconciliation loop. Any deployment relying on custom routes inside containers will not work.
|
||
|
||
---
|
||
|
||
### D9 — `HOST=true` Mode Not Implemented
|
||
|
||
**Shell script**: when `HOST=true`, uses `/sbin/iptables -t nat` directly on the host (without nsenter) for PREROUTING/POSTROUTING rules.
|
||
|
||
**Go implementation**: all host-level iptables operations use `m.run()` (which calls the detected binary directly), which is equivalent. However, there is no explicit `HOST` mode concept — the Go implementation always uses the host binary for host rules and nsenter for container rules. The distinction is implicit in whether `GetContainerPID` succeeds.
|
||
|
||
**Impact**: Functionally equivalent for the common case, but the explicit `HOST=true` override path (which bypasses nsenter even when a container name is set) has no equivalent in the Go implementation.
|
||
|
||
---
|
||
|
||
### D10 — Retry Logic for Name Resolution Not Implemented
|
||
|
||
**Shell script**: `name_resolver` with `STRICK_CHECK` retries up to `RETRIES_NUMBER` times (default 2) with a 1-second sleep if not enough containers are running.
|
||
|
||
**Go implementation**: `resolver.Resolver` has a `retries` field (default 2) and `SetRetries()` method, but the `Resolve()` method **does not implement any retry loop**. The retries field is set but never used.
|
||
|
||
**Impact**: In the Go implementation, if a container is not yet running when reconciliation runs, the name will fail to resolve immediately with no retry. The `WaitForContainerRunning` in `reconcileIPs()` provides some mitigation for container connections, but not for policy name resolution.
|
||
|
||
---
|
||
|
||
### D11 — `getLineNumbers` May Match Header Lines
|
||
|
||
**Shell script**: uses `awk '{print $1}'` after `grep` to extract line numbers. The `iptables --line-number` output has a header line (`Chain FORWARD (policy ACCEPT)` and column headers) that `grep` naturally filters out.
|
||
|
||
**Go implementation** (`getLineNumbers`):
|
||
```go
|
||
fields := strings.Fields(line)
|
||
if len(fields) > 0 {
|
||
matchingLines = append(matchingLines, fields[0])
|
||
}
|
||
```
|
||
This extracts the first field of every matching line. If a grep pattern accidentally matches a header line (e.g. a comment containing the word "Chain"), the first field of the header (`Chain`) would be added to `matchingLines` and passed to `iptables -D CHAIN Chain`, causing an iptables error.
|
||
|
||
**Impact**: Low probability but possible if comments contain words that appear in iptables output headers. The error would be non-fatal (logged) but could leave stale rules.
|
||
|
||
---
|
||
|
||
### D12 — `deleteMatchingLines` Reversal Strategy Difference
|
||
|
||
**Shell script**: uses `tac` to reverse the list of line numbers before deleting, ensuring highest-numbered lines are deleted first (preventing index shifting).
|
||
|
||
**Go implementation**:
|
||
```go
|
||
for i := len(lines) - 1; i >= 0; i-- {
|
||
if err := m.DeleteLine(chain, lines[i]); err != nil { ... }
|
||
}
|
||
```
|
||
This iterates the `lines` slice in reverse. However, `getLineNumbers` appends lines in the order they appear in `iptables` output (ascending line number order). Reversing the slice correctly deletes from highest to lowest.
|
||
|
||
**Impact**: Functionally correct. No bug, but worth noting the implementation differs from the shell script's `tac` approach.
|
||
|
||
---
|
||
|
||
### D13 — `EnsureEstablishedRelated` Checks Both Words Independently
|
||
|
||
**Shell script**:
|
||
```sh
|
||
if $IPTABLES -w -n --list $CHAIN | grep ESTABLISHED | grep RELATED | grep ACCEPT; then
|
||
```
|
||
This requires all three words to appear on the same line.
|
||
|
||
**Go implementation**:
|
||
```go
|
||
if !strings.Contains(string(output), "ESTABLISHED") || !strings.Contains(string(output), "RELATED") {
|
||
```
|
||
This checks if either word is absent **anywhere in the entire output**, not necessarily on the same line. If `ESTABLISHED` appears in one rule and `RELATED` in another, the Go implementation would consider the check passed even though no single rule covers both states.
|
||
|
||
**Impact**: The Go implementation may skip inserting the combined `ESTABLISHED,RELATED` rule if the chain already has separate rules for each state. This is an edge case but could result in missing the combined rule.
|
||
|
||
---
|
||
|
||
### D14 — `EXTRA_OPTIONS` / Debug Toggle via CLI Args Not Supported
|
||
|
||
**Shell script**: debug mode can be enabled by passing `"debug"` as a positional argument (`$2`, `$3`, or `$4`), which is captured in `EXTRA_OPTIONS`.
|
||
|
||
**Go implementation**: debug mode is controlled only by the `DEBUG` environment variable (`"1"` or `"true"`). There is no CLI argument support.
|
||
|
||
**Impact**: Minor operational difference; no functional impact on firewall behavior.
|
||
|
||
---
|
||
|
||
### D15 — `SOURCE_IP_FOR_POSTROUTING` Subnet Derivation Difference
|
||
|
||
**Shell script** (`postrouting()`):
|
||
```sh
|
||
SOURCE_IP_FOR_POSTROUTING="$(echo $SOURCE_IP | cut -d . -f1-3).0/24"
|
||
```
|
||
This always derives a `/24` from the source IP, regardless of the actual subnet mask.
|
||
|
||
**Go implementation** (`config.NetworkPrefix`):
|
||
```go
|
||
return fmt.Sprintf("%d.%d.%d.0/24", ipv4[0], ipv4[1], ipv4[2])
|
||
```
|
||
Identical behavior. However, `InsertPostroutingMasquerade` is never called (see D3), so this function's correctness is moot in practice.
|
||
|
||
---
|
||
|
||
### Summary Table
|
||
|
||
| ID | Category | Shell Script | Go Implementation | Severity |
|
||
|---|---|---|---|---|
|
||
| D1 | Name Resolution | Hosts file + Docker CLI | JSON config only | **High** |
|
||
| D2 | Role Filtering | `STRICK_CHECK` + `ROLES` label | Not implemented | **High** |
|
||
| D3 | POSTROUTING | Called when `POSTROUTING=true` | Never called | **High** |
|
||
| D4 | PREROUTING on Interface | `-t nat` always specified | Missing `-t nat` | **High** |
|
||
| D5 | Parameter Naming | `SOURCE_IP` used as `-d` | Same, but misleading name | Low |
|
||
| D6 | Container PREROUTING | No `-d` filter | Adds `-d 0.0.0.0/0` | Low |
|
||
| D7 | DELETE Operation | `OPERATION=DELETE` supported | Not implemented | **Medium** |
|
||
| D8 | Route Injection | `ROUTE=true` supported | Method exists, never called | **High** |
|
||
| D9 | HOST Mode | `HOST=true` explicit override | Implicit, no override | Low |
|
||
| D10 | Retry Logic | Retries with sleep | Field exists, not used | **Medium** |
|
||
| D11 | Line Number Extraction | `grep` filters headers | May match header lines | Low |
|
||
| D12 | Deletion Order | `tac` reverse | Slice reverse (correct) | None |
|
||
| D13 | ESTABLISHED/RELATED Check | Per-line grep | Whole-output contains | Low |
|
||
| D14 | Debug Toggle | CLI arg or env var | Env var only | Low |
|
||
| D15 | Subnet Derivation | `cut -d . -f1-3` | `NetworkPrefix()` | None (D3 makes it moot) |
|