Basic mapping so that iptables can still create valid tcp rules

JBPhoenix commented 2 years ago

Description

When reading protocol information from openvpn config files, remap to "tcp-client" to "tcp".

Motivation and Context

Openvpn configs specify protocol as either udp, tcp-client or tcp-server iptables doesn't know what to do with "tcp-client" and spits out this message: iptables v1.8.7 (legacy): unknown protocol "tcp-client" specified Doesn't halt the script but it does not add the intended iptables rule as-is. tcp-server shouldn't be specified in a client's configuration and won't need handling, AFAICT. From what I've read UDP is preferred & TCP isn't generally recommended for OpenVPN but acceptable as a fallback if UDP fails. I assume that if someone was connecting solely via TCP the container would be unable to make the outbound connection.

How Has This Been Tested?

Built a container locally with and without the changes.

Before changes iptables --list outputs:

Chain INPUT (policy DROP)
target     prot opt source               destination
ACCEPT     all  --  anywhere             anywhere             ctstate RELATED,ESTABLISHED
ACCEPT     all  --  anywhere             anywhere
ACCEPT     all  --  172.30.0.0/16        anywhere
ACCEPT     all  --  anywhere             anywhere

Chain FORWARD (policy DROP)
target     prot opt source               destination

Chain OUTPUT (policy DROP)
target     prot opt source               destination
ACCEPT     all  --  anywhere             anywhere
ACCEPT     all  --  anywhere             172.30.0.0/16
ACCEPT     udp  --  anywhere             remotehost             udp dpt:openvpn
ACCEPT     all  --  anywhere             anywhere

With my change:

Chain INPUT (policy DROP)
target     prot opt source               destination
ACCEPT     all  --  anywhere             anywhere             ctstate RELATED,ESTABLISHED
ACCEPT     all  --  anywhere             anywhere
ACCEPT     all  --  172.30.0.0/16        anywhere
ACCEPT     all  --  anywhere             anywhere

Chain FORWARD (policy DROP)
target     prot opt source               destination

Chain OUTPUT (policy DROP)
target     prot opt source               destination
ACCEPT     all  --  anywhere             anywhere
ACCEPT     all  --  anywhere             172.30.0.0/16
ACCEPT     udp  --  anywhere             remotehost             udp dpt:openvpn
ACCEPT     tcp  --  anywhere             remotehost             tcp dpt:https
ACCEPT     all  --  anywhere             anywhere

Key difference being: ACCEPT tcp -- anywhere remotehost tcp dpt:https

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist

[ ] I have updated the documentation accordingly.
[x] I have read the CONTRIBUTING document.
[ ] I have added tests to cover my changes if appropriate.
[ ] All new and existing tests passed.

Probably doesn't need a documentation update beyond the comment? I couldn't find any tests.

wfg commented 2 years ago

Do you have a config file that gets choked on? This line should already be trimming tcp-client to tcp: https://github.com/wfg/docker-openvpn-client/blob/b11a502a1c7db14b3ee7cb0200c7965c16ec39fa/data/scripts/entry.sh#L99

wfg commented 2 years ago

Actually, you got me thinking that we may also have an issue if a remote line had udp4/udp6/tcp4/tcp6 instead of just udp/tcp. It looks possible according to the OpenVPN manual.

JBPhoenix commented 2 years ago

First 3 lines of the config look like this:

client
remote example.com 1194 udp
remote example.com 443 tcp-client

No separate proto line, so not caught by line 99 I guess? Good catch on udp4/udp6/tcp4/tcp6 and none of these remote lines are inside a <connection> block either, which would be another wrinkle maybe?

wfg / docker-openvpn-client