ssh: T2635: migrate to get_config_dict()

Jinja template contains some workarounds like {% if port is string %}, this
depends of the resolution of https://phabricator.vyos.net/T2636

3 files changed, 93 insertions, 153 deletions

diff --git a/data/templates/ssh/sshd_config.tmpl b/data/templates/ssh/sshd_config.tmpl
index 08fe56655..1c136bb23 100644
--- a/data/templates/ssh/sshd_config.tmpl
+++ b/data/templates/ssh/sshd_config.tmpl
@@ -1,6 +1,10 @@
 ### Autogenerated by ssh.py ###
 
+# https://linux.die.net/man/5/sshd_config
+
+#
 # Non-configurable defaults
+#
 Protocol 2
 HostKey /etc/ssh/ssh_host_rsa_key
 HostKey /etc/ssh/ssh_host_dsa_key
@@ -22,99 +26,89 @@ TCPKeepAlive yes
 Banner /etc/issue.net
 Subsystem sftp /usr/lib/openssh/sftp-server
 UsePAM yes
+PermitRootLogin no
+
+#
+# User configurable section
+#
 
-# Specifies whether sshd should look up the remote host name,
-# and to check that the resolved host name for the remote IP
+# Look up remote host name and check that the resolved host name for the remote IP
 # address maps back to the very same IP address.
-UseDNS {{ host_validation }}
+UseDNS {{ "no" if disable_host_validation is defined else "yes" }}
 
-# Specifies the port number that sshd listens on. The default is 22.
-# Multiple options of this type are permitted.
-{% for p in port %}
-Port {{ p }}
-{% endfor %}
+# Specifies the port number that sshd(8) listens on
+{% if port is string %}
+Port {{ port }}
+{% else %}
+{%   for value in port %}
+Port {{ value }}
+{%   endfor %}
+{% endif %}
 
 # Gives the verbosity level that is used when logging messages from sshd
-LogLevel {{ log_level }}
-
-# Specifies whether root can log in using ssh
-PermitRootLogin no
+LogLevel {{ loglevel }}
 
 # Specifies whether password authentication is allowed
-PasswordAuthentication {{ password_authentication }}
+PasswordAuthentication {{ "no" if disable_password_authentication is defined else "yes" }}
 
-{% if listen_on %}
+{% if listen_address %}
 # Specifies the local addresses sshd should listen on
-{% for a in listen_on %}
-ListenAddress {{ a }}
-{% endfor %}
-{{ "\n" }}
-{% endif %}
-
-{%- if ciphers %}
-# Specifies the ciphers allowed. Multiple ciphers must be comma-separated.
-#
-# NOTE: As of now, there is no 'multi' node for 'ciphers', thus we have only one :/
-Ciphers {{ ciphers | join(",") }}
-{{ "\n" }}
-{% endif %}
-
-{%- if mac %}
-# Specifies the available MAC (message authentication code) algorithms. The MAC
-# algorithm is used for data integrity protection. Multiple algorithms must be
-# comma-separated.
-#
-# NOTE: As of now, there is no 'multi' node for 'mac', thus we have only one :/
-MACs {{ mac | join(",") }}
-{{ "\n" }}
-{% endif %}
-
-{%- if key_exchange %}
-# Specifies the available KEX (Key Exchange) algorithms. Multiple algorithms must
-# be comma-separated.
-#
-# NOTE: As of now, there is no 'multi' node for 'key-exchange', thus we have only one :/
-KexAlgorithms {{ key_exchange | join(",") }}
-{{ "\n" }}
+{%   if listen_address is string %}
+ListenAddress {{ listen_address }}
+{%   else %}
+{%     for address in listen_address %}
+ListenAddress {{ value }}
+{%     endfor %}
+{%   endif %}
 {% endif %}
 
-{%- if allow_users %}
-# This keyword can be followed by a list of user name patterns, separated by spaces.
-# If specified, login is allowed only for user names that match one of the patterns.
-# Only user names are valid, a numerical user ID is not recognized.
-AllowUsers {{ allow_users | join(" ") }}
-{{ "\n" }}
+{% if ciphers %}
+# Specifies the ciphers allowed for protocol version 2
+{% set value = ciphers if ciphers is string else ciphers | join(',') %}
+Ciphers {{ value }}
 {% endif %}
 
-{%- if allow_groups %}
-# This keyword can be followed by a list of group name patterns, separated by spaces.
-# If specified, login is allowed only for users whose primary group or supplementary
-# group list matches one of the patterns. Only group names are valid, a numerical group
-# ID is not recognized.
-AllowGroups {{ allow_groups | join(" ") }}
-{{ "\n" }}
+{% if mac %}
+# Specifies the available MAC (message authentication code) algorithms
+{% set value = mac if mac is string else mac | join(',') %}
+MACs {{ value }}
 {% endif %}
 
-{%- if deny_users %}
-# This keyword can be followed by a list of user name patterns, separated by spaces.
-# Login is disallowed for user names that match one of the patterns. Only user names
-# are valid, a numerical user ID is not	recognized.
-DenyUsers {{ deny_users | join(" ") }}
-{{ "\n" }}
+{% if key_exchange %}
+# Specifies the available Key Exchange algorithms
+{% set value = key_exchange if key_exchange is string else key_exchange | join(',') %}
+KexAlgorithms {{ value }}
 {% endif %}
 
-{%- if deny_groups %}
-# This keyword can be followed by a list of group name patterns, separated by spaces.
+{% if access_control is defined %}
+{%   if access_control.allow is defined %}
+{%     if access_control.allow.user is defined %}
+# If specified, login is allowed only for user names that match
+{% set value = access_control.allow.user if access_control.allow.user is string else access_control.allow.user | join(' ') %}
+AllowUsers {{ value }}
+{%     endif %}
+{%     if access_control.allow.group is defined %}
+# If specified, login is allowed only for users whose primary group or supplementary group list matches
+{% set value = access_control.allow.group if access_control.allow.group is string else access_control.allow.group | join(' ') %}
+AllowGroups {{ value }}
+{%     endif %}
+{%   endif %}
+{%   if access_control.deny is defined %}
+{%     if access_control.deny.user is defined %}
+# Login is disallowed for user names that match
+{% set value = access_control.deny.user if access_control.deny.user is string else access_control.deny.user | join(' ') %}
+DenyUsers {{ value }}
+{%     endif %}
+{%     if access_control.deny.group is defined %}
 # Login is disallowed for users whose primary group or supplementary group list matches
-# one of the patterns. Only group names are valid, a numerical group ID is not recognized.
-DenyGroups {{ deny_groups | join(" ") }}
-{{ "\n" }}
+{% set value = access_control.deny.group if access_control.deny.group is string else access_control.deny.group | join(' ') %}
+DenyGroups {{ value }}
+{%     endif %}
+{%   endif %}
 {% endif %}
 
-{%- if client_keepalive %}
+{% if client_keepalive_interval %}
 # Sets a timeout interval in seconds after which if no data has been received from the client,
-# sshd will send a message through the encrypted channel to request a response from the client.
-# The default is 0, indicating that these messages will not be sent to the client.
-# This option applies to protocol version 2 only.
-ClientAliveInterval {{ client_keepalive }}
+# sshd(8) will send a message through the encrypted channel to request a response from the client
+ClientAliveInterval {{ client_keepalive_interval }}
 {% endif %}
diff --git a/interface-definitions/ssh.xml.in b/interface-definitions/ssh.xml.in
index 4adfaecfb..1b20f5776 100644
--- a/interface-definitions/ssh.xml.in
+++ b/interface-definitions/ssh.xml.in
@@ -131,6 +131,9 @@
           <leafNode name="loglevel">
             <properties>
               <help>Log level</help>
+              <completionHelp>
+                <list>QUIET FATAL ERROR INFO VERBOSE</list>
+              </completionHelp>
               <valueHelp>
                 <format>QUIET</format>
                 <description>stay silent</description>
@@ -151,6 +154,9 @@
                 <format>VERBOSE</format>
                 <description>enable logging of failed login attempts</description>
               </valueHelp>
+              <constraint>
+                <regex>^(QUIET|FATAL|ERROR|INFO|VERBOSE)$</regex>
+              </constraint>
             </properties>
             <defaultValue>INFO</defaultValue>
           </leafNode>
@@ -179,10 +185,15 @@
                 <validator name="numeric" argument="--range 1-65535"/>
               </constraint>
             </properties>
+            <defaultValue>22</defaultValue>
           </leafNode>
           <leafNode name="client-keepalive-interval">
             <properties>
-              <help>how often send keep alives in seconds</help>
+              <help>Enable transmission of keepalives from server to client</help>
+              <valueHelp>
+                <format>1-65535</format>
+                <description>Time interval in seconds for keepalive message</description>
+              </valueHelp>
               <constraint>
                 <validator name="numeric" argument="--range 1-65535"/>
               </constraint>
diff --git a/src/conf_mode/ssh.py b/src/conf_mode/ssh.py
index 43fa2ff39..1ca2c8b4c 100755
--- a/src/conf_mode/ssh.py
+++ b/src/conf_mode/ssh.py
@@ -20,89 +20,28 @@ from netifaces import interfaces
 from sys import exit
 
 from vyos.config import Config
+from vyos.configdict import dict_merge
 from vyos import ConfigError
 from vyos.util import call
 from vyos.template import render
-
+from vyos.xml import defaults
 from vyos import airbag
 airbag.enable()
 
 config_file = r'/etc/ssh/sshd_config'
 systemd_override = r'/etc/systemd/system/ssh.service.d/override.conf'
 
-default_config_data = {
-    'port' : ['22'],
-    'log_level': 'INFO',
-    'password_authentication': 'yes',
-    'host_validation': 'yes',
-    'vrf': ''
-}
-
 def get_config():
-    ssh = default_config_data
     conf = Config()
     base = ['service', 'ssh']
     if not conf.exists(base):
         return None
-    else:
-        conf.set_level(base)
-
-    tmp = ['access-control', 'allow', 'user']
-    if conf.exists(tmp):
-        ssh['allow_users'] = conf.return_values(tmp)
-
-    tmp = ['access-control', 'allow', 'group']
-    if conf.exists(tmp):
-        ssh['allow_groups'] = conf.return_values(tmp)
-
-    tmp = ['access-control', 'deny' 'user']
-    if conf.exists(tmp):
-        ssh['deny_users'] = conf.return_values(tmp)
-
-    tmp = ['access-control', 'deny', 'group']
-    if conf.exists(tmp):
-        ssh['deny_groups'] = conf.return_values(tmp)
-
-    tmp = ['ciphers']
-    if conf.exists(tmp):
-        ssh['ciphers'] = conf.return_values(tmp)
-
-    tmp = ['key-exchange']
-    if conf.exists(tmp):
-        ssh['key_exchange'] = conf.return_values(tmp)
-
-    if conf.exists(['disable-host-validation']):
-        ssh['host_validation'] = 'no'
-
-    if conf.exists(['disable-password-authentication']):
-        ssh['password_authentication'] = 'no'
-
-    tmp = ['listen-address']
-    if conf.exists(tmp):
-        # We can listen on both IPv4 and IPv6 addresses
-        # Maybe there could be a check in the future if the configured IP address
-        # is configured on this system at all?
-        ssh['listen_on'] = conf.return_values(tmp)
-
-    tmp = ['loglevel']
-    if conf.exists(tmp):
-        ssh['log_level'] = conf.return_value(tmp)
-
-    tmp = ['mac']
-    if conf.exists(tmp):
-        ssh['mac'] = conf.return_values(tmp)
 
-    tmp = ['port']
-    if conf.exists(tmp):
-        ssh['port'] = conf.return_values(tmp)
-
-    tmp = ['client-keepalive-interval']
-    if conf.exists(tmp):
-        ssh['client_keepalive'] = conf.return_value(tmp)
-
-    tmp = ['vrf']
-    if conf.exists(tmp):
-        ssh['vrf'] = conf.return_value(tmp)
+    ssh = conf.get_config_dict(base, key_mangling=('-', '_'))
+    # We have gathered the dict representation of the CLI, but there are default
+    # options which we need to update into the dictionary retrived.
+    default_values = defaults(base)
+    ssh = dict_merge(default_values, ssh)
 
     return ssh
 
@@ -110,18 +49,18 @@ def verify(ssh):
     if not ssh:
         return None
 
-    if 'loglevel' in ssh.keys():
-        allowed_loglevel = 'QUIET, FATAL, ERROR, INFO, VERBOSE'
-        if not ssh['loglevel'] in allowed_loglevel:
-            raise ConfigError('loglevel must be one of "{0}"\n'.format(allowed_loglevel))
-
-    if ssh['vrf'] and ssh['vrf'] not in interfaces():
+    if 'vrf' in ssh.keys() and ssh['vrf'] not in interfaces():
         raise ConfigError('VRF "{vrf}" does not exist'.format(**ssh))
 
     return None
 
 def generate(ssh):
     if not ssh:
+        if os.path.isfile(config_file):
+            os.unlink(config_file)
+        if os.path.isfile(systemd_override):
+            os.unlink(systemd_override)
+
         return None
 
     render(config_file, 'ssh/sshd_config.tmpl', ssh, trim_blocks=True)
@@ -133,10 +72,6 @@ def apply(ssh):
     if not ssh:
         # SSH access is removed in the commit
         call('systemctl stop ssh.service')
-        if os.path.isfile(config_file):
-            os.unlink(config_file)
-        if os.path.isfile(systemd_override):
-            os.unlink(systemd_override)
 
     # Reload systemd manager configuration
     call('systemctl daemon-reload')