summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChristian Poessinger <christian@poessinger.com>2021-01-07 23:22:58 +0100
committerChristian Poessinger <christian@poessinger.com>2021-01-07 23:23:40 +0100
commitdcdc4f3ea27f1a26f8baa6b72b51c7911f21e6ba (patch)
treeba424b887b447fdc92e44fd52df13c02b33b2608
parent65ee3a66077c7708f366d9492033634024887545 (diff)
downloadvyos-1x-dcdc4f3ea27f1a26f8baa6b72b51c7911f21e6ba.tar.gz
vyos-1x-dcdc4f3ea27f1a26f8baa6b72b51c7911f21e6ba.zip
ssh: T2635: harden Jinja2 template and daemon startup
-rw-r--r--data/templates/ssh/sshd_config.tmpl30
-rwxr-xr-xsmoketest/scripts/cli/test_service_ssh.py12
-rwxr-xr-xsrc/conf_mode/ssh.py5
3 files changed, 22 insertions, 25 deletions
diff --git a/data/templates/ssh/sshd_config.tmpl b/data/templates/ssh/sshd_config.tmpl
index 1dc700d38..7d7257cae 100644
--- a/data/templates/ssh/sshd_config.tmpl
+++ b/data/templates/ssh/sshd_config.tmpl
@@ -48,59 +48,59 @@ LogLevel {{ loglevel | upper }}
# Specifies whether password authentication is allowed
PasswordAuthentication {{ "no" if disable_password_authentication is defined else "yes" }}
-{% if listen_address %}
+{% if listen_address is defined and listen_address is not none %}
# Specifies the local addresses sshd should listen on
{% for address in listen_address %}
ListenAddress {{ address }}
{% endfor %}
{% endif %}
-{% if ciphers %}
+{% if ciphers is defined and ciphers is not none %}
# Specifies the ciphers allowed for protocol version 2
-{% set value = ciphers if ciphers is string else ciphers | join(',') %}
+{% set value = ciphers if ciphers is string else ciphers | join(',') %}
Ciphers {{ value }}
{% endif %}
-{% if mac %}
+{% if mac is defined and mac is not none %}
# Specifies the available MAC (message authentication code) algorithms
-{% set value = mac if mac is string else mac | join(',') %}
+{% set value = mac if mac is string else mac | join(',') %}
MACs {{ value }}
{% endif %}
-{% if key_exchange %}
+{% if key_exchange is defined and key_exchange is not none %}
# Specifies the available Key Exchange algorithms
-{% set value = key_exchange if key_exchange is string else key_exchange | join(',') %}
+{% set value = key_exchange if key_exchange is string else key_exchange | join(',') %}
KexAlgorithms {{ value }}
{% endif %}
-{% if access_control is defined %}
-{% if access_control.allow is defined %}
+{% if access_control is defined and access_control is not none %}
+{% if access_control.allow is defined and access_control.allow is not none %}
{% 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(' ') %}
+{% 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(' ') %}
+{% 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 is defined and access_control.deny is not none %}
{% 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(' ') %}
+{% 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
-{% set value = access_control.deny.group if access_control.deny.group is string else access_control.deny.group | join(' ') %}
+{% 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_interval %}
+{% if client_keepalive_interval is defined and client_keepalive_interval is not none %}
# Sets a timeout interval in seconds after which if no data has been received from the client,
# sshd(8) will send a message through the encrypted channel to request a response from the client
ClientAliveInterval {{ client_keepalive_interval }}
diff --git a/smoketest/scripts/cli/test_service_ssh.py b/smoketest/scripts/cli/test_service_ssh.py
index e9e4639fc..eede042de 100755
--- a/smoketest/scripts/cli/test_service_ssh.py
+++ b/smoketest/scripts/cli/test_service_ssh.py
@@ -44,11 +44,6 @@ class TestServiceSSH(unittest.TestCase):
def tearDown(self):
# delete testing SSH config
self.session.delete(base_path)
- # restore "plain" SSH access
- self.session.set(base_path)
- # delete VRF
- self.session.delete(['vrf', 'name', vrf])
-
self.session.commit()
del self.session
@@ -109,7 +104,7 @@ class TestServiceSSH(unittest.TestCase):
def test_ssh_multiple_listen_addresses(self):
# Check if SSH service can be configured and runs with multiple
# listen ports and listen-addresses
- ports = ['22', '2222']
+ ports = ['22', '2222', '2223', '2224']
for port in ports:
self.session.set(base_path + ['port', port])
@@ -143,7 +138,7 @@ class TestServiceSSH(unittest.TestCase):
with self.assertRaises(ConfigSessionError):
self.session.commit()
- self.session.set(['vrf', 'name', vrf, 'table', '1001'])
+ self.session.set(['vrf', 'name', vrf, 'table', '1338'])
# commit changes
self.session.commit()
@@ -159,5 +154,8 @@ class TestServiceSSH(unittest.TestCase):
tmp = cmd(f'ip vrf pids {vrf}')
self.assertIn(PROCESS_NAME, tmp)
+ # delete VRF
+ self.session.delete(['vrf', 'name', vrf])
+
if __name__ == '__main__':
unittest.main(verbosity=2)
diff --git a/src/conf_mode/ssh.py b/src/conf_mode/ssh.py
index 07c057fd7..28e606663 100755
--- a/src/conf_mode/ssh.py
+++ b/src/conf_mode/ssh.py
@@ -68,6 +68,8 @@ def generate(ssh):
render(config_file, 'ssh/sshd_config.tmpl', ssh)
render(systemd_override, 'ssh/override.conf.tmpl', ssh)
+ # Reload systemd manager configuration
+ call('systemctl daemon-reload')
return None
@@ -76,9 +78,6 @@ def apply(ssh):
# SSH access is removed in the commit
call('systemctl stop ssh.service')
- # Reload systemd manager configuration
- call('systemctl daemon-reload')
-
if ssh:
call('systemctl restart ssh.service')