openvpn: T2234: clean up and rework openvpn template

- rearranged options to put them in logical groups separated by blank
  lines
- removed unnecessary blank lines (whitespace)
- fixed encryption if-else comparison logic that caused 3des to be
  ignored
- set tls if tls-version-min is set

2 files changed, 93 insertions, 83 deletions

diff --git a/data/templates/openvpn/server.conf.tmpl b/data/templates/openvpn/server.conf.tmpl
index e7715dfb5..5f6d1fc3c 100644
--- a/data/templates/openvpn/server.conf.tmpl
+++ b/data/templates/openvpn/server.conf.tmpl
@@ -3,18 +3,20 @@
 # See https://community.openvpn.net/openvpn/wiki/Openvpn24ManPage
 # for individual keyword definition
 
-{% if description %}
+{% if description -%}
 # {{ description }}
-{% endif %}
+
+{% endif -%}
 
 verb 3
 status /opt/vyatta/etc/openvpn/status/{{ intf }}.status 30
 writepid /var/run/openvpn/{{ intf }}.pid
 
-dev-type {{ type }}
-dev {{ intf }}
 user {{ uid }}
 group {{ gid }}
+
+dev-type {{ type }}
+dev {{ intf }}
 persist-key
 iproute /usr/libexec/vyos/system/unpriv-ip
 
@@ -22,187 +24,188 @@ proto {% if 'tcp-active' in protocol -%}tcp-client{% elif 'tcp-passive' in proto
 
 {%- if local_host %}
 local {{ local_host }}
-{% endif %}
+{%- endif %}
 
 {%- if mode == 'server' and protocol == 'udp' and not local_host %}
 multihome
-{% endif %}
+{%- endif %}
 
 {%- if local_port %}
 lport {{ local_port }}
-{% endif %}
+{%- endif %}
 
-{%- if remote_port %}
+{% if remote_port -%}
 rport {{ remote_port }}
 {% endif %}
 
 {%- if remote_host %}
-{% for remote in remote_host -%}
+{%- for remote in remote_host -%}
 remote {{ remote }}
 {% endfor -%}
-{% endif %}
+{% endif -%}
 
-{%- if shared_secret_file %}
+{% if shared_secret_file %}
 secret {{ shared_secret_file }}
-{% endif %}
+{%- endif %}
 
 {%- if persistent_tunnel %}
 persist-tun
-{% endif %}
+{%- endif %}
+
+{%- if redirect_gateway %}
+push "redirect-gateway {{ redirect_gateway }}"
+{%- endif %}
 
-{%- if mode %}
-{%- if 'client' in mode %}
+{%- if compress_lzo %}
+compress lzo
+{%- endif %}
+
+{% if 'client' in mode -%}
 #
 # OpenVPN Client mode
 #
 client
 nobind
-{%- elif 'server' in mode %}
+
+{% elif 'server' in mode -%}
 #
 # OpenVPN Server mode
 #
-mode server
-tls-server
-keepalive {{ ping_interval }} {{ ping_restart }}
-management /tmp/openvpn-mgmt-intf unix
 
 {%- if server_topology %}
 topology {% if 'point-to-point' in server_topology %}p2p{% else %}subnet{% endif %}
-{% endif %}
-
-{% for ns in server_dns_nameserver -%}
-push "dhcp-option DNS {{ ns }}"
-{% endfor -%}
-
-{% for route in server_push_route -%}
-push "route {{ route }}"
-{% endfor -%}
-
-{%- if server_domain %}
-push "dhcp-option DOMAIN {{ server_domain }}"
-{% endif %}
-
-{%- if server_max_conn %}
-max-clients {{ server_max_conn }}
-{% endif %}
+{%- endif %}
 
 {%- if bridge_member %}
 server-bridge nogw
 {%- else %}
 server {{ server_subnet }}
-{% endif %}
+{%- endif %}
+
+{%- if server_max_conn %}
+max-clients {{ server_max_conn }}
+{%- endif %}
 
 {%- if server_reject_unconfigured %}
 ccd-exclusive
+{%- endif %}
+
+keepalive {{ ping_interval }} {{ ping_restart }}
+management /tmp/openvpn-mgmt-intf unix
+
+{% for route in server_push_route -%}
+push "route {{ route }}"
+{% endfor -%}
+
+{% for ns in server_dns_nameserver -%}
+push "dhcp-option DNS {{ ns }}"
+{% endfor -%}
+
+{%- if server_domain -%}
+push "dhcp-option DOMAIN {{ server_domain }}"
 {% endif %}
 
-{%- else %}
+{% else -%}
 #
 # OpenVPN site-2-site mode
 #
 ping {{ ping_interval }}
 ping-restart {{ ping_restart }}
 
-{%- if local_address_subnet %}
+{% if local_address_subnet -%}
 ifconfig {{ local_address }} {{ local_address_subnet }}
-{% elif remote_address %}
+{%- elif remote_address -%}
 ifconfig {{ local_address }} {{ remote_address }}
-{% endif %}
+{%- endif %}
 
-{% endif %}
-{% endif %}
+{% endif -%}
 
+{% if tls -%}
+# TLS options
 {%- if tls_ca_cert %}
 ca {{ tls_ca_cert }}
-{% endif %}
+{%- endif %}
 
 {%- if tls_cert %}
 cert {{ tls_cert }}
-{% endif %}
+{%- endif %}
 
 {%- if tls_key %}
 key {{ tls_key }}
-{% endif %}
+{%- endif %}
 
 {%- if tls_crypt %}
 tls-crypt {{ tls_crypt }}
-{% endif %}
+{%- endif %}
 
 {%- if tls_crl %}
 crl-verify {{ tls_crl }}
-{% endif %}
+{%- endif %}
 
 {%- if tls_version_min %}
 tls-version-min {{tls_version_min}}
-{% endif %}
+{%- endif %}
 
 {%- if tls_dh %}
 dh {{ tls_dh }}
-{% endif %}
+{%- endif %}
 
 {%- if tls_auth %}
 tls-auth {{tls_auth}}
-{% endif %}
+{%- endif %}
 
+{%- if tls_role %}
 {%- if 'active' in tls_role %}
 tls-client
 {%- elif 'passive' in tls_role %}
 tls-server
-{% endif %}
+{%- endif %}
+{%- endif %}
 
-{%- if redirect_gateway %}
-push "redirect-gateway {{ redirect_gateway }}"
-{% endif %}
-
-{%- if compress_lzo %}
-compress lzo
-{% endif %}
-
-{%- if hash %}
-auth {{ hash }}
-{% endif %}
+{%- endif %}
 
+# Encryption options
 {%- if encryption %}
-{%- if 'des' in encryption %}
+{% if encryption == 'des' -%}
 cipher des-cbc
-{%- elif '3des' in encryption %}
+{%- elif encryption == '3des' -%}
 cipher des-ede3-cbc
-{%- elif 'bf128' in encryption %}
+{%- elif encryption == 'bf128' -%}
 cipher bf-cbc
 keysize 128
-{%- elif 'bf256' in encryption %}
+{%- elif encryption == 'bf256' -%}
 cipher bf-cbc
 keysize 25
-{%- elif 'aes128gcm' in encryption %}
+{%- elif encryption == 'aes128gcm' -%}
 cipher aes-128-gcm
-{%- elif 'aes128' in encryption %}
+{%- elif encryption == 'aes128' -%}
 cipher aes-128-cbc
-{%- elif 'aes192gcm' in encryption %}
+{%- elif encryption == 'aes192gcm' -%}
 cipher aes-192-gcm
-{%- elif 'aes192' in encryption %}
+{%- elif encryption == 'aes192' -%}
 cipher aes-192-cbc
-{%- elif 'aes256gcm' in encryption %}
+{%- elif encryption == 'aes256gcm' -%}
 cipher aes-256-gcm
-{%- elif 'aes256' in encryption %}
+{%- elif encryption == 'aes256' -%}
 cipher aes-256-cbc
-{% endif %}
-{% endif %}
+{%- endif -%}
+{%- endif %}
 
 {%- if ncp_ciphers %}
 ncp-ciphers {{ncp_ciphers}}
-{% endif %}
+{%- endif %}
 {%- if disable_ncp %}
 ncp-disable
-{% endif %}
+{%- endif %}
+
+{% if hash -%}
+auth {{ hash }}
+{%- endif -%}
 
 {%- if auth %}
 auth-user-pass /tmp/openvpn-{{ intf }}-pw
 auth-retry nointeract
-{% endif %}
-
-{%- if client %}
-client-config-dir /opt/vyatta/etc/openvpn/ccd/{{ intf }}
-{% endif %}
+{%- endif %}
 
 # DEPRECATED This option will be removed in OpenVPN 2.5
 # Until OpenVPN v2.3 the format of the X.509 Subject fields was formatted like this:
@@ -218,6 +221,12 @@ client-config-dir /opt/vyatta/etc/openvpn/ccd/{{ intf }}
 # See https://phabricator.vyos.net/T1512
 compat-names
 
+{% if options -%}
+#
+# Custom options added by user (not validated)
+#
+
 {% for option in options -%}
 {{ option }}
 {% endfor -%}
+{%- endif %}
diff --git a/src/conf_mode/interfaces-openvpn.py b/src/conf_mode/interfaces-openvpn.py
index e9b40bb38..f34e4f7fe 100755
--- a/src/conf_mode/interfaces-openvpn.py
+++ b/src/conf_mode/interfaces-openvpn.py
@@ -428,6 +428,7 @@ def get_config():
     # Minimum required TLS version
     if conf.exists('tls tls-version-min'):
          openvpn['tls_version_min'] = conf.return_value('tls tls-version-min')
+         openvpn['tls'] = True
 
     if conf.exists('shared-secret-key-file'):
         openvpn['shared_secret_file'] = conf.return_value('shared-secret-key-file')