add iptables rules cleanup in ovpn_run when the docker use host network

[jimlinntu]

Resolved the issue: #630

$ docker version
Client: Docker Engine - Community
 Version:           19.03.14
 API version:       1.40
 Go version:        go1.13.15
 Git commit:        5eb3275d40
 Built:             Tue Dec  1 19:20:17 2020
 OS/Arch:           linux/amd64
 Experimental:      false

Server: Docker Engine - Community
 Engine:
  Version:          19.03.14
  API version:      1.40 (minimum version 1.12)
  Go version:       go1.13.15
  Git commit:       5eb3275d40
  Built:            Tue Dec  1 19:18:45 2020
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.3.9
  GitCommit:        ea765aba0d05254012b0b9e595e995c09186427f
 runc:
  Version:          1.0.0-rc10
  GitCommit:        dc9208a3303feef5b3839f4323d9beb36df0a9dd
 docker-init:
  Version:          0.18.0
  GitCommit:        fec3683
$ docker-compose version
docker-compose version 1.27.4, build 40524192
docker-py version: 4.3.1
CPython version: 3.7.7
OpenSSL version: OpenSSL 1.1.0l  10 Sep 2019

docker-compose.yml I used to test.

version: "3.5"
services:
  ovpn:
    image: jimlin7777/openvpn
    command: "ovpn_run"
    cap_add:
      - NET_ADMIN
    volumes:
      - "./vpn-data:/etc/openvpn"
      - "./bin/ovpn_run:/usr/local/bin/ovpn_run"
    network_mode: "host"

You will find that after my commit, iptables will be restored to its original state.

(When the docker-openvpn is on)

$ iptables -t nat -L -v

Chain POSTROUTING (policy ACCEPT 30 packets, 2058 bytes)
 pkts bytes target     prot opt in     out     source               destination
  537 40652 MASQUERADE  all  --  any    !br-1b42953c3f82  172.20.0.0/16        anywhere
    6   388 MASQUERADE  all  --  any    !docker0  172.17.0.0/16        anywhere
    0     0 MASQUERADE  all  --  any    !docker_gwbridge  172.18.0.0/16        anywhere
  285 19769 MASQUERADE  all  --  any    !br-360ccd3b453f  192.168.48.0/20      anywhere
    0     0 MASQUERADE  tcp  --  any    any     172.17.0.2           172.17.0.2           tcp dpt:9998
    0     0 MASQUERADE  tcp  --  any    any     172.17.0.2           172.17.0.2           tcp dpt:9997
    0     0 MASQUERADE  tcp  --  any    any     172.17.0.2           172.17.0.2           tcp dpt:ssh
    0     0 MASQUERADE  tcp  --  any    any     172.20.0.3           172.20.0.3           tcp dpt:socks
    0     0 MASQUERADE  tcp  --  any    any     172.20.0.5           172.20.0.5           tcp dpt:http
    0     0 MASQUERADE  all  --  any    eth0    192.168.255.0/24     anywhere
    0     0 MASQUERADE  all  --  any    eth0    192.168.254.0/24     anywhere

(When the docker-openvpn is off)

$ iptables -t nat -L -v
Chain POSTROUTING (policy ACCEPT 123 packets, 8773 bytes)
 pkts bytes target     prot opt in     out     source               destination         
  534 40433 MASQUERADE  all  --  any    !br-1b42953c3f82  172.20.0.0/16        anywhere            
    6   388 MASQUERADE  all  --  any    !docker0  172.17.0.0/16        anywhere            
    0     0 MASQUERADE  all  --  any    !docker_gwbridge  172.18.0.0/16        anywhere            
  285 19769 MASQUERADE  all  --  any    !br-360ccd3b453f  192.168.48.0/20      anywhere            
    0     0 MASQUERADE  tcp  --  any    any     172.17.0.2           172.17.0.2           tcp dpt:9998
    0     0 MASQUERADE  tcp  --  any    any     172.17.0.2           172.17.0.2           tcp dpt:9997
    0     0 MASQUERADE  tcp  --  any    any     172.17.0.2           172.17.0.2           tcp dpt:ssh
    0     0 MASQUERADE  tcp  --  any    any     172.20.0.3           172.20.0.3           tcp dpt:socks
    0     0 MASQUERADE  tcp  --  any    any     172.20.0.5           172.20.0.5           tcp dpt:http

[kylemanna]

One of the main reasons this is an issue is that's not expected to run the container with networking mode = host. Why do that? At that point why not just use the distributions native container and use this image to manage the config files?

I'm sure there are other bad assumptions when run with host networking namespace.

That said, the implementation looks reasonably clean.

[jimlinntu]

Thanks for your fast reply!
Undeniably, the original ovpn_run is very simple and beautiful.
However, the reason I added this commit was because I saw this line (it explicitly allows the usage of the host network):

docker-openvpn/bin/ovpn_run

Lines 66 to 67 in 1228577

	# When using --net=host, use this to specify nat device.
	[ -z "$OVPN_NATDEVICE" ] && OVPN_NATDEVICE=eth0

I think unless ovpn_run strictly denies the usage of the host network,
I believe there are still people using this repository with host network mode without noticing that his/her iptables rules are not cleared up after this container down.
(maybe for the benefits that he/she can directly access the resource by the same IPs as he originally does on the host network)

[kylemanna]

Ahh yes, I see your reasoning, the original feature of endorsing host only behavior was a misstep on my part. My concern with these off nominal cases is that they have a long history of breaking because I don't personally use them and they aren't tested.

This PR highlights that point in that the code as originally written pollutes the iptables chains and there was no test to detect it.

Going forward I see two routes:

1. Drop all iptables features. I'd really rather the container not muck with firewall rules outside its network namespace. Furthmore, the world is (slowly) moving to nftables because of unified ipv6 support and higher performance. More iptables/ip6tables things are more tech debt at this point.
2. Write a CI test to verify the original feature and then verify that this PR fixes it and include the test. That way it's harder to break.

Can you share with me what host networking gets you over a proper docker network setup? From my experience, host networking often only helps those who don't understand Docker's complicated networking schemes.