From 2e943378d18b2eb56c6ae980fbe75dc59bc9b8b0 Mon Sep 17 00:00:00 2001 From: Sylvain Lamontagne Date: Thu, 22 Sep 2016 16:02:59 -0400 Subject: [PATCH 1/2] Too many arguments while pushing route So I was trying to push a route to my client and the script failed with 'too many arguments', I reworked this part and took the opportunity to rework a little bit the way push and routes were handled. I also added some tests and validated that what I changed would not break what was there before. --- bin/ovpn_genconfig | 66 ++++++++++++++++------------ test/tests/conf_options/container.sh | 55 ++++++++++++++++++++++- 2 files changed, 92 insertions(+), 29 deletions(-) diff --git a/bin/ovpn_genconfig b/bin/ovpn_genconfig index ca21514..1bba217 100755 --- a/bin/ovpn_genconfig +++ b/bin/ovpn_genconfig @@ -4,6 +4,10 @@ # Generate OpenVPN configs # +USE_DEFAULT_ROUTE=true + +TMP_PUSH_CONFIGFILE=$(mktemp -t vpn_push.XXXXXXX) +TMP_ROUTE_CONFIGFILE=$(mktemp -t vpn_route.XXXXXXX) TMP_EXTRA_CONFIGFILE=$(mktemp -t vpn_extra.XXXXXXX) #Traceback on Error and Exit come from https://docwhat.org/tracebacks-in-bash/ @@ -40,6 +44,8 @@ trap on_error ERR on_exit() { echo "Cleaning up before Exit ..." + rm -f $TMP_PUSH_CONFIGFILE + rm -f $TMP_ROUTE_CONFIGFILE rm -f $TMP_EXTRA_CONFIGFILE local _ec="$?" if [[ $_ec != 0 && "${_showed_traceback}" != t ]]; then @@ -99,11 +105,27 @@ usage() { echo " -z Enable comp-lzo compression." } +process_route_config() { + local ovpn_route_config='' + ovpn_route_config="$1" + # If user passed "0" skip this, assume no extra routes + [[ "$ovpn_route_config" == "0" ]] && break; + echo "Processing Route Config: '${ovpn_route_config}'" + [[ -n "$ovpn_route_config" ]] && echo "route $(getroute $ovpn_route_config)" >> "$TMP_ROUTE_CONFIGFILE" +} + +process_push_config() { + local ovpn_push_config='' + ovpn_push_config="$1" + echo "Processing PUSH Config: '${ovpn_push_config}'" + [[ -n "$ovpn_push_config" ]] && echo "push $ovpn_push_config" >> "$TMP_PUSH_CONFIGFILE" +} + process_extra_config() { local ovpn_extra_config='' ovpn_extra_config="$1" echo "Processing Extra Config: '${ovpn_extra_config}'" - [ -n "$ovpn_extra_config" ] && echo "$ovpn_extra_config" >> "$TMP_EXTRA_CONFIGFILE" + [[ -n "$ovpn_extra_config" ]] && echo "$ovpn_extra_config" >> "$TMP_EXTRA_CONFIGFILE" } @@ -127,10 +149,6 @@ OVPN_NAT=0 OVPN_DNS=1 OVPN_DEVICE="tun" OVPN_DEVICEN=0 -OVPN_ROUTES=() -TMP_ROUTES=() -OVPN_PUSH=() -TMP_PUSH=() OVPN_DNS_SERVERS=("8.8.8.8" "8.8.4.4") TMP_DNS_SERVERS=() OVPN_TLS_CIPHER='' @@ -157,7 +175,8 @@ while getopts ":a:e:C:T:r:s:du:cp:n:DNmf:tz2" opt; do OVPN_TLS_CIPHER="$OPTARG" ;; r) - TMP_ROUTES+=("$OPTARG") + USE_DEFAULT_ROUTE=false + process_route_config "$OPTARG" ;; s) OVPN_SERVER=$OPTARG @@ -172,7 +191,7 @@ while getopts ":a:e:C:T:r:s:du:cp:n:DNmf:tz2" opt; do OVPN_CLIENT_TO_CLIENT=1 ;; p) - TMP_PUSH+=("$OPTARG") + process_push_config "$OPTARG" ;; n) TMP_DNS_SERVERS+=("$OPTARG") @@ -216,12 +235,6 @@ done # Create ccd directory for static routes [ ! -d "${OPENVPN:-}/ccd" ] && mkdir -p ${OPENVPN:-}/ccd -# if new routes were not defined with -r, use default -[ ${#TMP_ROUTES[@]} -gt 0 ] && OVPN_ROUTES=("${TMP_ROUTES[@]}") - -# if new push directives were not defined with -p, use default -[ ${#TMP_PUSH[@]} -gt 0 ] && OVPN_PUSH=("${TMP_PUSH[@]}") - # if dns servers were not defined with -n, use google nameservers [ ${#TMP_DNS_SERVERS[@]} -gt 0 ] && OVPN_DNS_SERVERS=("${TMP_DNS_SERVERS[@]}") @@ -240,7 +253,7 @@ fi # Apply defaults [ -z "$OVPN_PROTO" ] && OVPN_PROTO=udp [ -z "$OVPN_PORT" ] && OVPN_PORT=1194 -[ ${#OVPN_ROUTES[@]} -eq 0 ] && OVPN_ROUTES=("192.168.254.0/24") +[ $USE_DEFAULT_ROUTE ] && process_route_config "192.168.254.0/24" export OVPN_SERVER OVPN_ROUTES OVPN_DEFROUTE export OVPN_SERVER_URL OVPN_ENV OVPN_PROTO OVPN_CN OVPN_PORT @@ -277,7 +290,6 @@ key-direction 0 keepalive 10 60 persist-key persist-tun -push block-outside-dns proto $OVPN_PROTO # Rely on Docker to do port mapping, internally always 1194 @@ -289,6 +301,9 @@ user nobody group nogroup EOF +#This was in the heredoc, we use the new function instead +process_push_config "block-outside-dns" + [ -n "$OVPN_TLS_CIPHER" ] && echo "tls-cipher $OVPN_TLS_CIPHER" >> "$conf" [ -n "$OVPN_CIPHER" ] && echo "cipher $OVPN_CIPHER" >> "$conf" [ -n "$OVPN_AUTH" ] && echo "auth $OVPN_AUTH" >> "$conf" @@ -299,22 +314,17 @@ EOF [ -n "${OVPN_FRAGMENT:-}" ] && echo "fragment $OVPN_FRAGMENT" >> "$conf" [ "$OVPN_DNS" == "1" ] && for i in "${OVPN_DNS_SERVERS[@]}"; do - echo "push dhcp-option DNS $i" >> "$conf" -done -# Append Routes -for i in "${OVPN_ROUTES[@]}"; do - # If user passed "0" skip this, assume no extra routes - [ "$i" = "0" ] && break; - echo route $(getroute "$i") >> "$conf" + process_push_config "push dhcp-option DNS $i" done +# Append route commands +echo -e "\n### Route Configurations Below" >> "$conf" +cat $TMP_ROUTE_CONFIGFILE >> "$conf" + # Append push commands -if [ ! -z ${OVPN_PUSH[@]:-} ];then - echo "${OVPN_PUSH}" - for i in "${OVPN_PUSH[@]}"; do - echo push \"$i\" >> "$conf" - done -fi +echo -e "\n### Push Configurations Below" >> "$conf" +cat $TMP_PUSH_CONFIGFILE >> "$conf" + # Optional OTP authentication support if [ -n "${OVPN_OTP_AUTH:-}" ]; then echo -e "\n\n# Enable OTP+PAM for user authentication" >> "$conf" diff --git a/test/tests/conf_options/container.sh b/test/tests/conf_options/container.sh index dbb8cd5..82fdc8a 100644 --- a/test/tests/conf_options/container.sh +++ b/test/tests/conf_options/container.sh @@ -13,7 +13,7 @@ max-clients 10 EOF SERV_IP=$(ip -4 -o addr show scope global | awk '{print $4}' | sed -e 's:/.*::' | head -n1) -ovpn_genconfig -u udp://$SERV_IP -f 1400 -e "$MULTILINE_EXTRA_SERVER_CONF" -e "duplicate-cn" -e "topology subnet" +ovpn_genconfig -u udp://$SERV_IP -f 1400 -e "$MULTILINE_EXTRA_SERVER_CONF" -e 'duplicate-cn' -e 'topology subnet' -p 'route 172.22.22.0 255.255.255.0' # # grep for config lines from openvpn.conf @@ -45,6 +45,21 @@ CONFIG_MATCH_DUPCN=$(busybox grep duplicate-cn /etc/openvpn/openvpn.conf) CONFIG_REQUIRED_TOPOLOGY="^topology subnet" CONFIG_MATCH_TOPOLOGY=$(busybox grep 'topology subnet' /etc/openvpn/openvpn.conf) +## Tests for push config +# 7. push route +CONFIG_REQUIRED_PUSH_ROUTE="^push route 172.22.22.0 255.255.255.0" +CONFIG_MATCH_PUSH_ROUTE=$(busybox grep 'push route 172.22.22.0 255.255.255.0' /etc/openvpn/openvpn.conf) + +## Test for default +# 8. Should see default route if none provided +CONFIG_REQUIRED_DEFAULT_ROUTE="^route 192.168.254.0 255.255.255.0" +CONFIG_MATCH_DEFAULT_ROUTE=$(busybox grep 'route 192.168.254.0 255.255.255.0' /etc/openvpn/openvpn.conf) + +# 9. Should see a push of 'block-outside-dns' by default +CONFIG_REQUIRED_DEFAULT_ROUTE="^push block-outside-dns" +CONFIG_MATCH_DEFAULT_ROUTE=$(busybox grep 'push block-outside-dns' /etc/openvpn/openvpn.conf) + + # # Tests # @@ -91,3 +106,41 @@ then else abort "==> Config match not found: $CONFIG_REQUIRED_TOPOLOGY != $CONFIG_MATCH_TOPOLOGY" fi + +if [[ $CONFIG_MATCH_PUSH_ROUTE =~ $CONFIG_REQUIRED_PUSH_ROUTE ]] +then + echo "==> Config match found: $CONFIG_REQUIRED_PUSH_ROUTE == $CONFIG_MATCH_PUSH_ROUTE" +else + abort "==> Config match not found: $CONFIG_REQUIRED_PUSH_ROUTE != $CONFIG_MATCH_PUSH_ROUTE" +fi + +if [[ $CONFIG_MATCH_DEFAULT_ROUTE =~ $CONFIG_REQUIRED_DEFAULT_ROUTE ]] +then + echo "==> Config match found: $CONFIG_REQUIRED_DEFAULT_ROUTE == $CONFIG_MATCH_DEFAULT_ROUTE" +else + abort "==> Config match not found: $CONFIG_REQUIRED_DEFAULT_ROUTE != $CONFIG_MATCH_DEFAULT_ROUTE" +fi + + +SERV_IP=$(ip -4 -o addr show scope global | awk '{print $4}' | sed -e 's:/.*::' | head -n1) +ovpn_genconfig -u udp://$SERV_IP -r "172.33.33.0/24" -r "172.34.34.0/24" + +CONFIG_REQUIRED_ROUTE_1="^route 172.33.33.0 255.255.255.0" +CONFIG_MATCH_ROUTE_1=$(busybox grep 'route 172.33.33.0 255.255.255.0' /etc/openvpn/openvpn.conf) + +CONFIG_REQUIRED_ROUTE_2="^route 172.34.34.0 255.255.255.0" +CONFIG_MATCH_ROUTE_2=$(busybox grep 'route 172.34.34.0 255.255.255.0' /etc/openvpn/openvpn.conf) + +if [[ $CONFIG_MATCH_ROUTE_1 =~ $CONFIG_REQUIRED_ROUTE_1 ]] +then + echo "==> Config match found: $CONFIG_REQUIRED_ROUTE_1 == $CONFIG_MATCH_ROUTE_1" +else + abort "==> Config match not found: $CONFIG_REQUIRED_ROUTE_1 != $CONFIG_MATCH_ROUTE_1" +fi + +if [[ $CONFIG_MATCH_ROUTE_2 =~ $CONFIG_REQUIRED_ROUTE_2 ]] +then + echo "==> Config match found: $CONFIG_REQUIRED_ROUTE_2 == $CONFIG_MATCH_ROUTE_2" +else + abort "==> Config match not found: $CONFIG_REQUIRED_ROUTE_2 != $CONFIG_MATCH_ROUTE_2" +fi From 72a3c8a0011630675c3916c33e6d006244b3499e Mon Sep 17 00:00:00 2001 From: Sylvain Lamontagne Date: Thu, 22 Sep 2016 18:12:45 -0400 Subject: [PATCH 2/2] Fix for regression As I reworked the push options, a bug got introduced where a duplication of push in the config for the DNS dhcp-options would make it to fail. There was no tests covering this, so I did not catch it earlier. I've add the missing tests and fix the bug --- bin/ovpn_genconfig | 2 +- test/tests/conf_options/container.sh | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/bin/ovpn_genconfig b/bin/ovpn_genconfig index 1bba217..a8b595f 100755 --- a/bin/ovpn_genconfig +++ b/bin/ovpn_genconfig @@ -314,7 +314,7 @@ process_push_config "block-outside-dns" [ -n "${OVPN_FRAGMENT:-}" ] && echo "fragment $OVPN_FRAGMENT" >> "$conf" [ "$OVPN_DNS" == "1" ] && for i in "${OVPN_DNS_SERVERS[@]}"; do - process_push_config "push dhcp-option DNS $i" + process_push_config "dhcp-option DNS $i" done # Append route commands diff --git a/test/tests/conf_options/container.sh b/test/tests/conf_options/container.sh index 82fdc8a..b3f210f 100644 --- a/test/tests/conf_options/container.sh +++ b/test/tests/conf_options/container.sh @@ -59,6 +59,12 @@ CONFIG_MATCH_DEFAULT_ROUTE=$(busybox grep 'route 192.168.254.0 255.255.255.0' /e CONFIG_REQUIRED_DEFAULT_ROUTE="^push block-outside-dns" CONFIG_MATCH_DEFAULT_ROUTE=$(busybox grep 'push block-outside-dns' /etc/openvpn/openvpn.conf) +# 10. Should see a push of 'dhcp-option DNS' by default +CONFIG_REQUIRED_DEFAULT_DNS_1="^push dhcp-option DNS 8.8.8.8" +CONFIG_MATCH_DEFAULT_DNS_1=$(busybox grep 'push dhcp-option DNS 8.8.8.8' /etc/openvpn/openvpn.conf) +CONFIG_REQUIRED_DEFAULT_DNS_2="^push dhcp-option DNS 8.8.4.4" +CONFIG_MATCH_DEFAULT_DNS_2=$(busybox grep 'push dhcp-option DNS 8.8.4.4' /etc/openvpn/openvpn.conf) + # # Tests @@ -121,6 +127,19 @@ else abort "==> Config match not found: $CONFIG_REQUIRED_DEFAULT_ROUTE != $CONFIG_MATCH_DEFAULT_ROUTE" fi +if [[ $CONFIG_MATCH_DEFAULT_DNS_1 =~ $CONFIG_REQUIRED_DEFAULT_DNS_1 ]] +then + echo "==> Config match found: $CONFIG_REQUIRED_DEFAULT_DNS_1 == $CONFIG_MATCH_DEFAULT_DNS_1" +else + abort "==> Config match not found: $CONFIG_REQUIRED_DEFAULT_DNS_1 != $CONFIG_MATCH_DEFAULT_DNS_1" +fi + +if [[ $CONFIG_MATCH_DEFAULT_DNS_2 =~ $CONFIG_REQUIRED_DEFAULT_DNS_2 ]] +then + echo "==> Config match found: $CONFIG_REQUIRED_DEFAULT_DNS_2 == $CONFIG_MATCH_DEFAULT_DNS_2" +else + abort "==> Config match not found: $CONFIG_REQUIRED_DEFAULT_DNS_2 != $CONFIG_MATCH_DEFAULT_DNS_2" +fi SERV_IP=$(ip -4 -o addr show scope global | awk '{print $4}' | sed -e 's:/.*::' | head -n1) ovpn_genconfig -u udp://$SERV_IP -r "172.33.33.0/24" -r "172.34.34.0/24"