Merge pull request #267 from Jakuje/runtime

fix: Review and update service units and socket unit to include distribution defaults
willshersystems · Jan 25, 2024 · cb8c339 · cb8c339
2 parents 32b892c + f59b40b
commit cb8c339
Show file tree

Hide file tree

Showing 29 changed files with 375 additions and 39 deletions.
diff --git a/.github/workflows/ansible-debian-check.yml b/.github/workflows/ansible-debian-check.yml
@@ -15,6 +15,7 @@ jobs:
           group: local
           hosts: localhost
           targets: "tests/tests_*.yml"
+          requirements: tests/requirements.yml
 
   debian-bullseye:
     runs-on: ubuntu-latest
@@ -28,6 +29,7 @@ jobs:
           group: local
           hosts: localhost
           targets: "tests/tests_*.yml"
+          requirements: tests/requirements.yml
 
   debian-buster:
     runs-on: ubuntu-latest
@@ -41,3 +43,4 @@ jobs:
           group: local
           hosts: localhost
           targets: "tests/tests_*.yml"
+          requirements: tests/requirements.yml
diff --git a/.github/workflows/ansible-ubuntu.yml b/.github/workflows/ansible-ubuntu.yml
@@ -16,6 +16,7 @@ jobs:
           group: local
           hosts: localhost
           targets: "tests/tests_*.yml"
+          requirements: tests/requirements.yml
 
   ubuntu-20:
     runs-on: ubuntu-latest
@@ -30,3 +31,4 @@ jobs:
           group: local
           hosts: localhost
           targets: "tests/tests_*.yml"
+          requirements: tests/requirements.yml
diff --git a/handlers/main.yml b/handlers/main.yml
@@ -6,7 +6,7 @@
     state: reloaded
   when:
     - sshd_allow_reload|bool
-    - ansible_facts['virtualization_type']|default(None) not in __sshd_skip_virt_env
+    - ansible_facts['virtualization_type'] | default(None) not in __sshd_skip_virt_env
     - ansible_connection != 'chroot'
     - ansible_facts['os_family'] != 'AIX'
     - ansible_facts['os_family'] != 'OpenWrt'

diff --git a/tasks/find_ports.yml b/tasks/find_ports.yml
@@ -23,4 +23,4 @@
   when:
     - sshd_manage_firewall | bool or sshd_manage_selinux | bool
     - ansible_facts['os_family'] == 'RedHat'
-    - ansible_facts['virtualization_type']|default(None) not in __sshd_skip_virt_env
+    - ansible_facts['virtualization_type'] | default(None) not in __sshd_skip_virt_env
diff --git a/tasks/install.yml b/tasks/install.yml
@@ -132,14 +132,14 @@
         - sshd_manage_firewall | bool
         - ansible_facts['os_family'] == 'RedHat'
         - ansible_facts['distribution_version'] is version('7', '>=')
-        - ansible_facts['virtualization_type']|default(None) not in __sshd_skip_virt_env
+        - ansible_facts['virtualization_type'] | default(None) not in __sshd_skip_virt_env
 
     - name: Configure selinux
       ansible.builtin.include_tasks: selinux.yml
       when:
         - sshd_manage_selinux | bool
         - ansible_facts['os_family'] == 'RedHat'
-        - ansible_facts['virtualization_type']|default(None) not in __sshd_skip_virt_env
+        - ansible_facts['virtualization_type'] | default(None) not in __sshd_skip_virt_env
 
     - name: Create the complete configuration file
       ansible.builtin.include_tasks: install_config.yml

diff --git a/tasks/install_service.yml b/tasks/install_service.yml
@@ -10,6 +10,7 @@
         group: root
         mode: "0644"
       notify: reload_sshd
+
     - name: Install instanced service unit file
       ansible.builtin.template:
         src: "{{ sshd_service_template_at_service }}"
@@ -18,6 +19,9 @@
         group: root
         mode: "0644"
       notify: reload_sshd
+      when:
+        - __sshd_socket_accept | bool
+
     - name: Install socket unit file
       ansible.builtin.template:
         src: "{{ sshd_service_template_socket }}"
@@ -34,7 +38,7 @@
     state: started
   when:
     - sshd_manage_service|bool
-    - ansible_facts['virtualization_type']|default(None) not in __sshd_skip_virt_env
+    - ansible_facts['virtualization_type'] | default(None) not in __sshd_skip_virt_env
     - ansible_connection != 'chroot'
 
 # Due to ansible bug 21026, cannot use service module on RHEL 7
@@ -43,5 +47,5 @@
   when:
     - ansible_connection == 'chroot'
     - ansible_facts['os_family'] == 'RedHat'
-    - ansible_facts['distribution_major_version']|int >= 7
+    - ansible_facts['distribution_major_version'] | int >= 7
   changed_when: true
diff --git a/templates/sshd.service.j2 b/templates/sshd.service.j2
@@ -1,15 +1,37 @@
 [Unit]
 Description=OpenBSD Secure Shell server
+After=network.target{{ (__sshd_service_after is none) | ternary('', ' ' ~ __sshd_service_after) }}
+{% if __sshd_service_wants is string %}
+Wants={{ __sshd_service_wants }}
+{% elif __sshd_service_wants is iterable %}
+{%   for file in __sshd_service_wants %}
+Wants={{ file }}
+{%   endfor %}
+{% endif %}
 Documentation=man:sshd(8) man:sshd_config(5)
 
 [Service]
 Type=notify
+{% if __sshd_environment_file is string %}
+EnvironmentFile=-{{ __sshd_environment_file }}
+{% elif __sshd_environment_file is iterable %}
+{%   for file in __sshd_environment_file %}
+EnvironmentFile=-{{ file }}
+{%   endfor %}
+{% endif %}
 ExecStartPre={{ sshd_binary }} -t
-ExecStart={{ sshd_binary }} -D -f {{ sshd_config_file }}
+ExecStart={{ sshd_binary }} -D {{ __sshd_environment_variable }} -f {% if sshd_main_config_file is not none %}
+{{- sshd_main_config_file }}
+{% else %}
+{{- sshd_config_file }}
+{% endif %}
 ExecReload={{ sshd_binary }} -t
 ExecReload=/bin/kill -HUP $MAINPID
 KillMode=process
 Restart=on-failure
+{% if __sshd_service_restart_timeout is not none %}
+RestartSec={{ __sshd_service_restart_timeout }}
+{% endif %}
 RestartPreventExitStatus=255
 {% if __sshd_runtime_directory is not none %}
 RuntimeDirectory={{ __sshd_runtime_directory }}
@@ -18,3 +40,6 @@ RuntimeDirectoryMode={{ __sshd_runtime_directory_mode }}
 
 [Install]
 WantedBy=multi-user.target
+{% if __sshd_service_alias is not none %}
+Alias={{ __sshd_service_alias }}.service
+{% endif %}
diff --git a/templates/sshd.socket.j2 b/templates/sshd.socket.j2
@@ -1,12 +1,19 @@
 [Unit]
 Description=OpenBSD Secure Shell server socket
 Documentation=man:sshd(8) man:sshd_config(5)
-Before={{ sshd_service }}.service
+{% if __sshd_socket_accept %}
 Conflicts={{ sshd_service }}.service
+{% else %}
+Before=sockets.target
+{% endif %}
 
 [Socket]
 ListenStream=22
+{% if __sshd_socket_accept %}
 Accept=yes
+{% else %}
+Accept=no
+{% endif %}
 
 [Install]
 WantedBy=sockets.target
diff --git a/templates/sshd@.service.j2 b/templates/sshd@.service.j2
@@ -1,12 +1,33 @@
 [Unit]
 Description=OpenBSD Secure Shell server per-connection daemon
 Documentation=man:sshd(8) man:sshd_config(5)
-After=auditd.service
+{% if __sshd_service_after is not none %}
+After={{ __sshd_service_after }}
+{% endif %}
+{% if __sshd_service_wants is string %}
+Wants={{ __sshd_service_wants }}
+{% elif __sshd_service_wants is iterable %}
+{%   for file in __sshd_service_wants %}
+Wants={{ file }}
+{%   endfor %}
+{% endif %}
 
 [Service]
-ExecStart=-{{ sshd_binary }} -i -f {{ sshd_config_file }}
+{% if __sshd_environment_file is string %}
+EnvironmentFile=-{{ __sshd_environment_file }}
+{% elif __sshd_environment_file is iterable %}
+{%   for file in __sshd_environment_file %}
+EnvironmentFile=-{{ file }}
+{%   endfor %}
+{% endif %}
+ExecStart=-{{ sshd_binary }} -i {{ __sshd_environment_variable }} -f {% if sshd_main_config_file is not none %}
+{{- sshd_main_config_file }}
+{% else %}
+{{- sshd_config_file }}
+{% endif %}
 StandardInput=socket
 {% if __sshd_runtime_directory is not none %}
 RuntimeDirectory={{ __sshd_runtime_directory }}
+RuntimeDirectoryPreserve=yes
 RuntimeDirectoryMode={{ __sshd_runtime_directory_mode }}
 {% endif %}
diff --git a/tests/tasks/restore.yml b/tests/tasks/restore.yml
@@ -38,6 +38,6 @@
   changed_when: false
   when:
     - __sshd_test_backup is defined
-    - ansible_facts['virtualization_type']|default(None) not in __sshd_skip_virt_env
+    - ansible_facts['virtualization_type'] | default(None) not in __sshd_skip_virt_env
     - ansible_connection != 'chroot'
     - ansible_facts['os_family'] != 'AIX'
diff --git a/tests/tasks/setup.yml b/tests/tasks/setup.yml
@@ -57,5 +57,5 @@
     main_sshd_config_name: 00-ansible_system_role.conf
     main_sshd_config_path: /etc/ssh/sshd_config.d/
   when:
-    - (ansible_facts['os_family'] == 'RedHat' and ansible_facts['distribution_major_version']|int > 8) or
-      (ansible_facts['distribution'] == 'Ubuntu' and ansible_facts['distribution_major_version']|int >= 22)
+    - (ansible_facts['os_family'] == 'RedHat' and ansible_facts['distribution_major_version'] | int > 8) or
+      (ansible_facts['distribution'] == 'Ubuntu' and ansible_facts['distribution_major_version'] | int >= 20)
diff --git a/tests/tests_all_options.yml b/tests/tests_all_options.yml
@@ -90,10 +90,10 @@
       ansible.builtin.shell: >-
         set -o pipefail && man sshd_config \
           | sed 's/\x08.//g' \
-          | grep -o '^     [A-Z][A-Za-z0-9]*\(.\|  \)' \
+          | grep -o '^\(     \|       \)[A-Z][A-Za-z0-9]*\(.\|  \)' \
           | grep -v "[A-Za-z0-9] $" | grep -v "[^A-Za-z0-9 ]$" \
           | awk '{ print $1 }' \
-          | grep -v '^$' | grep -v "^Match$"
+          | grep -v '^$' | grep -v "^\(Match\|OpenSSH\|The\|Arguments\|Theo\)$"
       args:
         executable: /bin/bash
       register: sshd_options
@@ -102,7 +102,7 @@
 
     - name: Print all the possible options
       ansible.builtin.debug:
-        var: ssh_options.stdout_lines
+        var: sshd_options.stdout_lines
 
     - name: Construct the configuration list
       ansible.builtin.set_fact:

diff --git a/tests/tests_alternative_file.yml b/tests/tests_alternative_file.yml
@@ -104,7 +104,7 @@
               - "'AuthorizedKeysFile .ssh/authorized_keys' in config.content | b64decode"
           when:
             - ansible_facts['os_family'] == 'RedHat'
-            - ansible_facts['distribution_major_version']|int > 8
+            - ansible_facts['distribution_major_version'] | int > 8
 
         - name: Check RHEL7 and RHEL8 defaults are present in the first configuration file
           ansible.builtin.assert:
@@ -114,8 +114,8 @@
               - "'UsePAM yes' in config.content | b64decode"
           when:
             - ansible_facts['os_family'] == 'RedHat'
-            - ansible_facts['distribution_major_version']|int > 6
-            - ansible_facts['distribution_major_version']|int < 9
+            - ansible_facts['distribution_major_version'] | int > 6
+            - ansible_facts['distribution_major_version'] | int < 9
 
         - name: Check RHEL6 defaults are present in the first configuration file
           ansible.builtin.assert:
@@ -133,17 +133,24 @@
               - "'UsePAM yes' in config.content | b64decode"
           when:
             - ansible_facts['os_family'] == 'Debian'
-            - ansible_facts['distribution_major_version']|int < 22
+            - ansible_facts['distribution_major_version'] | int < 22
 
-        - name: Check Ubuntu 22 defaults are present in the first configuration file
+        - name: Check Ubuntu 20+ defaults are present in the first configuration file
           ansible.builtin.assert:
             that:
               - "'Include /etc/ssh/sshd_config.d/*.conf' in config3.content | b64decode"
-              - "'KbdInteractiveAuthentication no' in config.content | b64decode"
               - "'UsePAM yes' in config.content | b64decode"
           when:
             - ansible_facts['distribution'] == 'Ubuntu'
-            - ansible_facts['distribution_major_version']|int >= 22
+            - ansible_facts['distribution_major_version'] | int >= 20
+
+        - name: Check Ubuntu 22+ defaults are present in the first configuration file
+          ansible.builtin.assert:
+            that:
+              - "'KbdInteractiveAuthentication no' in config.content | b64decode"
+          when:
+            - ansible_facts['distribution'] == 'Ubuntu'
+            - ansible_facts['distribution_major_version'] | int >= 22
 
         - name: Check content of second configuration file
           ansible.builtin.assert:
@@ -172,7 +179,7 @@
               - "'Subsystem sftp /usr/libexec/openssh/sftp-server' in config3.content | b64decode"
           when:
             - ansible_facts['os_family'] == 'RedHat'
-            - ansible_facts['distribution_major_version']|int > 8
+            - ansible_facts['distribution_major_version'] | int > 8
             - ansible_facts['distribution'] != 'Fedora'
 
         - name: Check the main configuration file contains some default values for Fedora
@@ -183,7 +190,7 @@
               - "'Subsystem sftp /usr/libexec/sftp-server' in config3.content | b64decode"
           when:
             - ansible_facts['os_family'] == 'RedHat'
-            - ansible_facts['distribution_major_version']|int > 8
+            - ansible_facts['distribution_major_version'] | int > 8
             - ansible_facts['distribution'] == 'Fedora'
 
         - name: Check the generated config has requested properties

diff --git a/tests/tests_alternative_file_role.yml b/tests/tests_alternative_file_role.yml
@@ -122,7 +122,7 @@
               - "'AuthorizedKeysFile .ssh/authorized_keys' in config.content | b64decode"
           when:
             - ansible_facts['os_family'] == 'RedHat'
-            - ansible_facts['distribution_major_version']|int > 8
+            - ansible_facts['distribution_major_version'] | int > 8
 
         - name: Check RHEL7 and RHEL8 defaults are present in the first configuration file
           ansible.builtin.assert:
@@ -132,8 +132,8 @@
               - "'UsePAM yes' in config.content | b64decode"
           when:
             - ansible_facts['os_family'] == 'RedHat'
-            - ansible_facts['distribution_major_version']|int > 6
-            - ansible_facts['distribution_major_version']|int < 9
+            - ansible_facts['distribution_major_version'] | int > 6
+            - ansible_facts['distribution_major_version'] | int < 9
 
         - name: Check RHEL6 defaults are present in the first configuration file
           ansible.builtin.assert:
@@ -151,18 +151,24 @@
               - "'UsePAM yes' in config.content | b64decode"
           when:
             - ansible_facts['os_family'] == 'Debian'
-            - ansible_facts['distribution_major_version']|int < 22
+            - ansible_facts['distribution_major_version'] | int < 22
 
-        - name: Check Ubuntu 22 defaults are present in the first configuration file
+        - name: Check Ubuntu 20+ defaults are present in the first configuration file
           ansible.builtin.assert:
             that:
               - "'Include /etc/ssh/sshd_config.d/*.conf' in config3.content | b64decode"
-              - "'KbdInteractiveAuthentication no' in config.content | b64decode"
               - "'UsePAM yes' in config.content | b64decode"
           when:
             - ansible_facts['distribution'] == 'Ubuntu'
-            - ansible_facts['distribution_major_version']|int >= 22
+            - ansible_facts['distribution_major_version'] | int >= 20
 
+        - name: Check Ubuntu 22+ defaults are present in the first configuration file
+          ansible.builtin.assert:
+            that:
+              - "'KbdInteractiveAuthentication no' in config.content | b64decode"
+          when:
+            - ansible_facts['distribution'] == 'Ubuntu'
+            - ansible_facts['distribution_major_version'] | int >= 22
 
         - name: Check content of second configuration file
           ansible.builtin.assert:
@@ -191,7 +197,7 @@
               - "'Subsystem sftp /usr/libexec/openssh/sftp-server' in config3.content | b64decode"
           when:
             - ansible_facts['os_family'] == 'RedHat'
-            - ansible_facts['distribution_major_version']|int > 8
+            - ansible_facts['distribution_major_version'] | int > 8
             - ansible_facts['distribution'] != 'Fedora'
 
         - name: Check the main configuration file contains some default values for Fedora
@@ -202,7 +208,7 @@
               - "'Subsystem sftp /usr/libexec/sftp-server' in config3.content | b64decode"
           when:
             - ansible_facts['os_family'] == 'RedHat'
-            - ansible_facts['distribution_major_version']|int > 8
+            - ansible_facts['distribution_major_version'] | int > 8
             - ansible_facts['distribution'] == 'Fedora'
 
         - name: Check the generated config has requested properties

diff --git a/tests/tests_include_present.yml b/tests/tests_include_present.yml
@@ -17,7 +17,7 @@
         state: absent
       when:
         - (ansible_facts['os_family'] == 'RedHat' and ansible_facts['distribution_major_version'] | int > 8) or
-          (ansible_facts['distribution'] == 'Ubuntu' and ansible_facts['distribution_major_version'] | int >= 22)
+          (ansible_facts['distribution'] == 'Ubuntu' and ansible_facts['distribution_major_version'] | int >= 20)
 
     - name: Create a new configuration in drop-in directory
       ansible.builtin.include_role:
@@ -29,12 +29,12 @@
           Ciphers: aes192-ctr
       when:
         - (ansible_facts['os_family'] == 'RedHat' and ansible_facts['distribution_major_version'] | int > 8) or
-          (ansible_facts['distribution'] == 'Ubuntu' and ansible_facts['distribution_major_version'] | int >= 22)
+          (ansible_facts['distribution'] == 'Ubuntu' and ansible_facts['distribution_major_version'] | int >= 20)
 
     - name: Verify the options are correctly set
       when:
         - (ansible_facts['os_family'] == 'RedHat' and ansible_facts['distribution_major_version'] | int > 8) or
-          (ansible_facts['distribution'] == 'Ubuntu' and ansible_facts['distribution_major_version'] | int >= 22)
+          (ansible_facts['distribution'] == 'Ubuntu' and ansible_facts['distribution_major_version'] | int >= 20)
       tags: tests::verify
       block:
         - name: Flush handlers