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

@@ -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"
+  process_push_config "push dhcp-option DNS $i"
-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"
 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 -e "\n### Push Configurations Below" >> "$conf"
-  echo "${OVPN_PUSH}"
+cat $TMP_PUSH_CONFIGFILE >> "$conf"
-  for i in "${OVPN_PUSH[@]}"; do
+
-    echo push \"$i\" >> "$conf"
-  done
-fi
 # Optional OTP authentication support
 if [ -n "${OVPN_OTP_AUTH:-}" ]; then
     echo -e "\n\n# Enable OTP+PAM for user authentication" >> "$conf"

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